* RE: [PATCH v4 1/14] input: cyapa: re-architecture driver to support multi-trackpads in one driver
From: Dudley Du @ 2014-09-25 5:53 UTC (permalink / raw)
To: 'Dmitry Torokhov'
Cc: Rafael J. Wysocki, Benson Leung, Patrik Fimml, linux-input,
linux-kernel
In-Reply-To: <20140925004838.GB38891@core.coreip.homeip.net>
> -----Original Message-----
> From: linux-input-owner@vger.kernel.org [mailto:linux-input-
> owner@vger.kernel.org] On Behalf Of Dmitry Torokhov
> Sent: Thursday, September 25, 2014 8:49 AM
> To: Dudley Du
> Cc: Rafael J. Wysocki; Benson Leung; Patrik Fimml; linux-input@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v4 1/14] input: cyapa: re-architecture driver to support
> multi-trackpads in one driver
>
> On Wed, Sep 24, 2014 at 03:08:31PM +0800, Dudley Du wrote:
> > > > > > + async_schedule(cyapa_detect_async, cyapa);
> > > > >
> > > > > I think I already asked this before - why do we need to try and
> schedule
> > > async
> > > > > detect from interrupt handler. In what cases we will fail to
> initialize
> > > the
> > > > > device during normal probing, but then are fine when stray interrupt
> > > arrives?
> > > > >
> > > >
> > > > 1) Because gen5 TP devices use interrupt to sync the command and
> response,
> > > and
> > > > in detecting, some commands must be sent and executed in attached
> devices,
> > > so
> > > > the interrupt must be usable in detecting.
> > > > So if do not schedule async detect from interrupt handler, the interrupt
> > > > handler will be taken, and cannot enter again, which will cause the
> command
> > > to
> > > > the device failed, it will also cause long time detecting.
> > > > 2) detecting will fail when no device attached or the attached device is
> not
> > > > supported gen3 or gen5 TP devices or there is some issue with the device
> > > > that cannot enter into active working mode or stay in bootloader mode
> > > > for invalid firmware detected.
> > > > And it's tested that it's save when stray interrupt arrives in detecting.
> > >
> > >
> > > I am sorry, I have trouble parsing this. I understand that you may need
> > > interrupt to know when command response is ready, but I do not see how
> > > kicking asynchronous detect helps there. During probe you can figure out
> > > if you are talking to a Cypress device and whether it is operable. If it
> > > is not operable you can try flashing a new firmware and then kick off
> > > detection again after flash is successful. But I do not see any reason
> > > in trying to re-detect the device after random interrupt arrives in hopes
> > > that maybe this time stars will align and we'll be able to work with it.
> >
> > Considering two situations:
> > 1) For some machines, when system get into sleep mode, the power of the
> trackpad device
> > will be cut off, and after system resumed, the power to trackpad device is
> on again.
>
> And cyapa_resume() is responsible for bringing the device back into
> operational state.
>
> > 2) For the trackpad device itself, if internal error encounter, it will
> reset itself,
> > similar result as power off and power on again.
> > When either of above situation happened, for gen3 Cypress trackpad device,
> it will get
> > into bootloader mode (it cannot get into operational mode automatically),
> and also assert
> > interrupts to host.
> > So at this time, cyapa driver must kick off the detection again to determine
> the real status
> > of the trackpad device, and command it go to the operational mode.
> > Otherwise, it will be out of work until reboot.
>
> In this case you won't have cyapa_default_irq_handler installed, right?
> The specific generation IRQ handler will have to detect this condition
> and reinitialize the touchpad.
Yes, you are correct.
The async detection will be removed from cyapa_default_irq_handler().
>
> ...
>
> > > > > > @@ -898,8 +528,12 @@ static int cyapa_remove(struct i2c_client
> *client)
> > > > > > struct cyapa *cyapa = i2c_get_clientdata(client);
> > > > > >
> > > > >
> > > > > You schedule detect asynchronously so you need to make sure it is not
> > > running
> > > > > (and will not be running) when you try to unbind the driver here.
> > > >
> > > > Thanks. I will update it in next release.
> > > >
> > > > To avoid this problem, a removed flag is introduced and is also synced
> by
> > > the
> > > > cyapa->state_sync_lock mutex lock. So when this driver is removing, it's
> > > sure
> > > > that the detecting asynchronous thread is not running, and if there's
> any
> > > event
> > > > cause the asynchronous thread is entered, it will do nothing and will
> exit
> > > immediately.
> > >
> > > Peeking at the other version of the patch you do:
> > >
> > > cyapa_remove()
> > > {
> > > ...
> > > cyapa->removed = true;
> > > ...
> > > kfree(cyapa);
> > > }
> > >
> > > and
> > >
> > > void cyapa_detect_async(void *data, async_cookie_t cookie)
> > > {
> > > ...
> > > if (cyapa->removed) {
> > > mutex_unlock(&cyapa->state_sync_lock);
> > > return;
> > > }
> > > ...
> > > }
> > >
> > > That's not going to work.
> > >
> > > Getting asynchronous behavior by individual driver is hard and I believe
> > > Luis Rordiguez is working on allowing to do that is the driver core. So
> > > I would recommend to switch driver to do synchronous probing, at least
> > > for now, and figure out other kinks first.
> > >
> > > Thanks.
> > >
> >
> > The consideration of the thread asynchronous issue is that:
> > The detection thread has been scheduled, but not run yet, and at the time,
> > the driver itself has been removed, so when later the detection thread is
> > triggered to run, the memory of cyapa driver has been freed, and cause error,
> right?
>
> Right.
>
> >
> > I have an idea as below, please help review.
> > Add variable cyapa->detect_thread_actived to trace the number of the threads
> were scheduled,
> > and when the driver module is in removing, it will wait all scheduled
> threads has been run.
> > That is when a thread is scheduled, cyapa->detect_thread_actived will be
> increased,
> > when the thread is executed and finished, the cyapa->detect_thread_actived
> will be decrreased,
> > so, when the value of cyapa->detect_thread_actived is 0, it must be no
> thread scheduled for running.
> > So when at this time, if the driver module is marked for removed, the
> remove() thread that waiting
> > on the cyapa->detect_thread_completed will be issue, and then driver is safe
> to be removed.
> > The code is similar as below.
> >
> > For each time, before the cyapa_detect_async() function is called or
> scheduled,
> > ... {
> > ...
> > atomic_inc(&cyapa->detect_thread_actived);
> > async_schedule(cyapa_detect_async, cyapa);
> > ...
> > }
> >
> > void cyapa_detect_async(void *data, async_cookie_t cookie)
> > {
> > ...
> > if (cyapa->removed) {
> > mutex_unlock(&cyapa->state_sync_lock);
> > return;
> > }
> > ...
> > free_irq(cyapa->irq, cyapa);
> >
> > /* Wait until all scheduled thread exited. */
> > if (atomic_read(&cyapa->detect_thread_actived)
> > wait_for_completion_timeout(&cyapa->detect_thread_completed,
> > msecs_to_jiffies(4000));
> > ...
> > }
> >
> > void cyapa_detect_async(void *data, async_cookie_t cookie)
> > {
> > struct cyapa *cyapa = (struct cyapa *)data;
> >
> > mutex_lock(&cyapa->state_sync_lock);
> > if (cyapa->removed) {
> > mutex_unlock(&cyapa->state_sync_lock);
> > if (atomic_dec_and_test(&cyapa->detect_thread_actived)) {
> > /* Now, no thread runing, safe to remove this driver. */
> > complete(&cyapa->detect_thread_completed);
> > }
> > goto return;
> > }
> >
> > /* Keep synchronized with sys interface process threads. */
> > cyapa_detect(cyapa);
> >
> > mutex_unlock(&cyapa->state_sync_lock);
> > atomic_dec(&cyapa->detect_thread_actived);
> > }
>
> It can be made to work, but as I mentioned we are working on bringing
> asynchronous probing into device core in one form or another so I'd
> rather not be doing it in the driver at all at this time.
Understood.
But what should I do for this driver, right now?
Though synchronous probing could be working anyway, but I do not suggest to use this method in this driver.
For this driver and devices, I have below concerns when using asynchronous probing:
1) Slow down the system boot time.
At system start up, it will take much long time for probing, so system start up will
be slowed down.
2) Polling mode for these device would be lower efficient and unreliable in detecting.
If using asynchronous detecting when error detected in irq handler, then must use polling mode.
But use polling mode will cause device easier to get into unstable status,
Fast polling will consume too much firmware CPU time, and may make it unstable.
Low polling requires much more time for device detecting.
And also the data/command read/write in polling mode is not well synchronized between HOST and device,
it may cause the device data become unreliable.
Thanks.
>
> Thanks.
>
> --
> Dmitry
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] hid: sony: Update the DualShock 4 touchpad resolution
From: Jiri Kosina @ 2014-09-25 9:25 UTC (permalink / raw)
To: Frank Praznik; +Cc: linux-input
In-Reply-To: <1411565899-4358-1-git-send-email-frank.praznik@oh.rr.com>
On Wed, 24 Sep 2014, Frank Praznik wrote:
> The DualShock 4 touchpad has been measured to have a resolution of
> 44.86 dots/mm which equates to 1920x942.
>
> Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: add new gamepad LED constants
From: David Herrmann @ 2014-09-25 10:26 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Michael Wright, open list:HID CORE LAYER, Michael Wright,
Jiri Kosina
In-Reply-To: <20140923163756.GC40700@core.coreip.homeip.net>
Hi
On Tue, Sep 23, 2014 at 6:37 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>> It's the same discussion we had with
>> backlights for years and we're trying eagerly to merge them into the
>> DRM char-devs.
>>
>
> So keyboard backlight is going to be controlled by display chardev? Or
> we are fragmenting backlights into different classes?
The idea is to make DRM property changes propagate into a linked
backlight. Applied to input-LEDs, it would mean setting LED_XYZ via
input would cause an kernel-internal call into the linked led_class
set_brightness(). This allows us to register an led-class per LED as
you request, but at the same time allow easy access via input devices
where it makes sense.
Thanks
David
^ permalink raw reply
* Re: How to indicate hover touch when exact distance unknown?
From: Chung-Yih Wang (王崇懿) @ 2014-09-26 4:07 UTC (permalink / raw)
To: linux-input
>>How about ABS_MT_PRESENT and/or ABS_PRESENT? It would complement TOUCH in the
>>case of hovering, allow the state where the tool is there but not touching, and
>>would unambiguously advertise the capability of detecting presence. It would
>>also be forward compatible with additional capabilities, such as reporting the
>>actual distance to the surface.
>>
>>Henrik
Or how about ABS_MT_TYPE which indicates touch type of a slot (finger,
hovering, stylus, ...)?
Chung-yih
^ permalink raw reply
* [PATCH 0/6] mfd: fix platform-device id collisions
From: Johan Hovold @ 2014-09-26 10:55 UTC (permalink / raw)
To: Samuel Ortiz, Lee Jones
Cc: Jiri Kosina, linux-input-u79uwXL29TY76Z2rM5mHXA,
Greg Kroah-Hartman, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Johan Hovold
Hot-pluggable multi-function devices should always be registered with
PLATFORM_DEVID_AUTO to avoid name collisions on the platform bus.
This series fix the two mfd drivers that currently fail to get this
right, and also adds a new helper function to assist any future driver
authors.
Included is also a fix of how mfd core generates the platform ids for
subdevices. Currently, the mfd cell-id is simply added to the id base
that mfd_add_devices is called with. This effectively prevents
mfd-devices from using PLATFORM_DEVID_AUTO (-2) while still having
non-zero cell ids.
In a different thread I mentioned that using for example
bus_num << 8 | dev_num
as an id-base for USB multi-function devices would also avoid any
collisions, but encoding the bus topology in the id base like this
is not good idea. [1] Not only would it force any new transports to come
up with unique id bases, it would also be very ad-hoc differ from driver
to driver (consider multi-interface USB devices or non-zero cell ids).
Note that if userspace needs to find sibling interfaces it should
never rely on device naming anyway, but rather use the topology already
encoded in sysfs.
The only thing that is currently not possible trough sysfs is to figure
out which sibling interface is which should they have the same name but
unique cell ids (consider an MFD with multiple leds or gpio chips).
Again, parsing device ids is not an option, but if needed we could
simply let driver core export any mfd cell-id for platform devices. Note
that the final patch is a pre-requisite for this.
Johan
[1] http://marc.info/?l=linux-kernel&m=141094514827834&w=2
Johan Hovold (6):
mfd: viperboard: fix platform-device id collision
mfd: rtsx_usb: fix platform device-id collision
mfd: core: add helper function to register hotplug devices
mfd: use mfd_add_hotplug_devices helper
HID: hid-sensor-hub: use mfd_add_hotplug_devices helper
mfd: core: fix platform-device id generation
drivers/hid/hid-sensor-hub.c | 8 +++-----
drivers/mfd/mfd-core.c | 8 +++++++-
drivers/mfd/rtsx_usb.c | 4 ++--
drivers/mfd/viperboard.c | 4 ++--
include/linux/mfd/core.h | 7 +++++++
5 files changed, 21 insertions(+), 10 deletions(-)
--
1.8.5.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 1/6] mfd: viperboard: fix platform-device id collision
From: Johan Hovold @ 2014-09-26 10:55 UTC (permalink / raw)
To: Samuel Ortiz, Lee Jones
Cc: Jiri Kosina, linux-input, Greg Kroah-Hartman, linux-kernel,
linux-usb, Johan Hovold
In-Reply-To: <1411728933-13351-1-git-send-email-johan@kernel.org>
Allow more than one viperboard to be connected by registering with
PLATFORM_DEVID_AUTO instead of PLATFORM_DEVID_NONE.
The subdevices are currently registered with PLATFORM_DEVID_NONE, which
will cause a name collision on the platform bus when a second viperboard
is plugged in:
viperboard 1-2.4:1.0: version 0.00 found at bus 001 address 004
------------[ cut here ]------------
WARNING: CPU: 0 PID: 181 at /home/johan/work/omicron/src/linux/fs/sysfs/dir.c:31 sysfs_warn_dup+0x74/0x84()
sysfs: cannot create duplicate filename '/bus/platform/devices/viperboard-gpio'
Modules linked in: i2c_viperboard viperboard netconsole [last unloaded: viperboard]
CPU: 0 PID: 181 Comm: bash Tainted: G W 3.17.0-rc6 #1
[<c0016bf4>] (unwind_backtrace) from [<c0013860>] (show_stack+0x20/0x24)
[<c0013860>] (show_stack) from [<c04305f8>] (dump_stack+0x24/0x28)
[<c04305f8>] (dump_stack) from [<c0040fb4>] (warn_slowpath_common+0x80/0x98)
[<c0040fb4>] (warn_slowpath_common) from [<c004100c>] (warn_slowpath_fmt+0x40/0x48)
[<c004100c>] (warn_slowpath_fmt) from [<c016f1bc>] (sysfs_warn_dup+0x74/0x84)
[<c016f1bc>] (sysfs_warn_dup) from [<c016f548>] (sysfs_do_create_link_sd.isra.2+0xcc/0xd0)
[<c016f548>] (sysfs_do_create_link_sd.isra.2) from [<c016f588>] (sysfs_create_link+0x3c/0x48)
[<c016f588>] (sysfs_create_link) from [<c02867ec>] (bus_add_device+0x12c/0x1e0)
[<c02867ec>] (bus_add_device) from [<c0284820>] (device_add+0x410/0x584)
[<c0284820>] (device_add) from [<c0289440>] (platform_device_add+0xd8/0x26c)
[<c0289440>] (platform_device_add) from [<c02a5ae4>] (mfd_add_device+0x240/0x344)
[<c02a5ae4>] (mfd_add_device) from [<c02a5ce0>] (mfd_add_devices+0xb8/0x110)
[<c02a5ce0>] (mfd_add_devices) from [<bf00d1c8>] (vprbrd_probe+0x160/0x1b0 [viperboard])
[<bf00d1c8>] (vprbrd_probe [viperboard]) from [<c030c000>] (usb_probe_interface+0x1bc/0x2a8)
[<c030c000>] (usb_probe_interface) from [<c028768c>] (driver_probe_device+0x14c/0x3ac)
[<c028768c>] (driver_probe_device) from [<c02879e4>] (__driver_attach+0xa4/0xa8)
[<c02879e4>] (__driver_attach) from [<c0285698>] (bus_for_each_dev+0x70/0xa4)
[<c0285698>] (bus_for_each_dev) from [<c0287030>] (driver_attach+0x2c/0x30)
[<c0287030>] (driver_attach) from [<c030a288>] (usb_store_new_id+0x170/0x1ac)
[<c030a288>] (usb_store_new_id) from [<c030a2f8>] (new_id_store+0x34/0x3c)
[<c030a2f8>] (new_id_store) from [<c02853ec>] (drv_attr_store+0x30/0x3c)
[<c02853ec>] (drv_attr_store) from [<c016eaa8>] (sysfs_kf_write+0x5c/0x60)
[<c016eaa8>] (sysfs_kf_write) from [<c016dc68>] (kernfs_fop_write+0xd4/0x194)
[<c016dc68>] (kernfs_fop_write) from [<c010fe40>] (vfs_write+0xb4/0x1c0)
[<c010fe40>] (vfs_write) from [<c01104a8>] (SyS_write+0x4c/0xa0)
[<c01104a8>] (SyS_write) from [<c000f900>] (ret_fast_syscall+0x0/0x48)
---[ end trace 98e8603c22d65817 ]---
viperboard 1-2.4:1.0: Failed to add mfd devices to core.
viperboard: probe of 1-2.4:1.0 failed with error -17
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/mfd/viperboard.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/mfd/viperboard.c b/drivers/mfd/viperboard.c
index e00f5340ed87..3c2b8f9e3c84 100644
--- a/drivers/mfd/viperboard.c
+++ b/drivers/mfd/viperboard.c
@@ -93,8 +93,9 @@ static int vprbrd_probe(struct usb_interface *interface,
version >> 8, version & 0xff,
vb->usb_dev->bus->busnum, vb->usb_dev->devnum);
- ret = mfd_add_devices(&interface->dev, -1, vprbrd_devs,
- ARRAY_SIZE(vprbrd_devs), NULL, 0, NULL);
+ ret = mfd_add_devices(&interface->dev, PLATFORM_DEVID_AUTO,
+ vprbrd_devs, ARRAY_SIZE(vprbrd_devs), NULL, 0,
+ NULL);
if (ret != 0) {
dev_err(&interface->dev, "Failed to add mfd devices to core.");
goto error;
--
1.8.5.5
^ permalink raw reply related
* [PATCH 2/6] mfd: rtsx_usb: fix platform device-id collision
From: Johan Hovold @ 2014-09-26 10:55 UTC (permalink / raw)
To: Samuel Ortiz, Lee Jones
Cc: Jiri Kosina, linux-input-u79uwXL29TY76Z2rM5mHXA,
Greg Kroah-Hartman, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Johan Hovold
In-Reply-To: <1411728933-13351-1-git-send-email-johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Hot-pluggable multi-function devices should use PLATFORM_DEVID_AUTO to
avoid name collisions on the platform bus.
This driver currently uses the USB-device address as an id. This makes
name collisions unlikely, but it could still happen if two devices are
connected to separate buses and gets assigned the same address.
Signed-off-by: Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
drivers/mfd/rtsx_usb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mfd/rtsx_usb.c b/drivers/mfd/rtsx_usb.c
index 71f387ce8cbd..78073e4b87e4 100644
--- a/drivers/mfd/rtsx_usb.c
+++ b/drivers/mfd/rtsx_usb.c
@@ -647,7 +647,7 @@ static int rtsx_usb_probe(struct usb_interface *intf,
/* initialize USB SG transfer timer */
setup_timer(&ucr->sg_timer, rtsx_usb_sg_timed_out, (unsigned long) ucr);
- ret = mfd_add_devices(&intf->dev, usb_dev->devnum, rtsx_usb_cells,
+ ret = mfd_add_devices(&intf->dev, PLATFORM_DEVID_AUTO, rtsx_usb_cells,
ARRAY_SIZE(rtsx_usb_cells), NULL, 0, NULL);
if (ret)
goto out_init_fail;
--
1.8.5.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 3/6] mfd: core: add helper function to register hotplug devices
From: Johan Hovold @ 2014-09-26 10:55 UTC (permalink / raw)
To: Samuel Ortiz, Lee Jones
Cc: Jiri Kosina, linux-input, Greg Kroah-Hartman, linux-kernel,
linux-usb, Johan Hovold
In-Reply-To: <1411728933-13351-1-git-send-email-johan@kernel.org>
Hot-pluggable multi-function devices should always be registered with
PLATFORM_DEVID_AUTO to avoid name collisions on the platform bus. This
helper also hides the memory map and irq parameters, which aren't used
by hot-pluggable (e.g. USB-based) devices.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
include/linux/mfd/core.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/include/linux/mfd/core.h b/include/linux/mfd/core.h
index f543de91ce19..1e47262a1c63 100644
--- a/include/linux/mfd/core.h
+++ b/include/linux/mfd/core.h
@@ -108,6 +108,13 @@ extern int mfd_add_devices(struct device *parent, int id,
struct resource *mem_base,
int irq_base, struct irq_domain *irq_domain);
+static inline int mfd_add_hotplug_devices(struct device *parent,
+ const struct mfd_cell *cells, int n_devs)
+{
+ return mfd_add_devices(parent, PLATFORM_DEVID_AUTO, cells, n_devs,
+ NULL, 0, NULL);
+}
+
extern void mfd_remove_devices(struct device *parent);
#endif
--
1.8.5.5
^ permalink raw reply related
* [PATCH 4/6] mfd: use mfd_add_hotplug_devices helper
From: Johan Hovold @ 2014-09-26 10:55 UTC (permalink / raw)
To: Samuel Ortiz, Lee Jones
Cc: Jiri Kosina, linux-input, Greg Kroah-Hartman, linux-kernel,
linux-usb, Johan Hovold
In-Reply-To: <1411728933-13351-1-git-send-email-johan@kernel.org>
Use mfd_add_hotplug_devices helper to register the subdevices.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/mfd/rtsx_usb.c | 4 ++--
drivers/mfd/viperboard.c | 5 ++---
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/mfd/rtsx_usb.c b/drivers/mfd/rtsx_usb.c
index 78073e4b87e4..5b17339fce7c 100644
--- a/drivers/mfd/rtsx_usb.c
+++ b/drivers/mfd/rtsx_usb.c
@@ -647,8 +647,8 @@ static int rtsx_usb_probe(struct usb_interface *intf,
/* initialize USB SG transfer timer */
setup_timer(&ucr->sg_timer, rtsx_usb_sg_timed_out, (unsigned long) ucr);
- ret = mfd_add_devices(&intf->dev, PLATFORM_DEVID_AUTO, rtsx_usb_cells,
- ARRAY_SIZE(rtsx_usb_cells), NULL, 0, NULL);
+ ret = mfd_add_hotplug_devices(&intf->dev, rtsx_usb_cells,
+ ARRAY_SIZE(rtsx_usb_cells));
if (ret)
goto out_init_fail;
diff --git a/drivers/mfd/viperboard.c b/drivers/mfd/viperboard.c
index 3c2b8f9e3c84..be450ebcb52d 100644
--- a/drivers/mfd/viperboard.c
+++ b/drivers/mfd/viperboard.c
@@ -93,9 +93,8 @@ static int vprbrd_probe(struct usb_interface *interface,
version >> 8, version & 0xff,
vb->usb_dev->bus->busnum, vb->usb_dev->devnum);
- ret = mfd_add_devices(&interface->dev, PLATFORM_DEVID_AUTO,
- vprbrd_devs, ARRAY_SIZE(vprbrd_devs), NULL, 0,
- NULL);
+ ret = mfd_add_hotplug_devices(&interface->dev, vprbrd_devs,
+ ARRAY_SIZE(vprbrd_devs));
if (ret != 0) {
dev_err(&interface->dev, "Failed to add mfd devices to core.");
goto error;
--
1.8.5.5
^ permalink raw reply related
* [PATCH 5/6] HID: hid-sensor-hub: use mfd_add_hotplug_devices helper
From: Johan Hovold @ 2014-09-26 10:55 UTC (permalink / raw)
To: Samuel Ortiz, Lee Jones
Cc: Jiri Kosina, linux-input, Greg Kroah-Hartman, linux-kernel,
linux-usb, Johan Hovold
In-Reply-To: <1411728933-13351-1-git-send-email-johan@kernel.org>
Use mfd_add_hotplug_devices helper to register the subdevices.
Compile-only tested.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/hid/hid-sensor-hub.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index 2ac25760a9a9..fbe1a6d2aa53 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -641,9 +641,6 @@ static int sensor_hub_probe(struct hid_device *hdev,
goto err_stop_hw;
}
sd->hid_sensor_hub_client_devs[
- sd->hid_sensor_client_cnt].id =
- PLATFORM_DEVID_AUTO;
- sd->hid_sensor_hub_client_devs[
sd->hid_sensor_client_cnt].name = name;
sd->hid_sensor_hub_client_devs[
sd->hid_sensor_client_cnt].platform_data =
@@ -659,8 +656,9 @@ static int sensor_hub_probe(struct hid_device *hdev,
if (last_hsdev)
last_hsdev->end_collection_index = i;
- ret = mfd_add_devices(&hdev->dev, 0, sd->hid_sensor_hub_client_devs,
- sd->hid_sensor_client_cnt, NULL, 0, NULL);
+ ret = mfd_add_hotplug_devices(&hdev->dev,
+ sd->hid_sensor_hub_client_devs,
+ sd->hid_sensor_client_cnt);
if (ret < 0)
goto err_stop_hw;
--
1.8.5.5
^ permalink raw reply related
* [PATCH 6/6] mfd: core: fix platform-device id generation
From: Johan Hovold @ 2014-09-26 10:55 UTC (permalink / raw)
To: Samuel Ortiz, Lee Jones
Cc: Jiri Kosina, linux-input, Greg Kroah-Hartman, linux-kernel,
linux-usb, Johan Hovold
In-Reply-To: <1411728933-13351-1-git-send-email-johan@kernel.org>
Make sure to always honour multi-function devices registered with
PLATFORM_DEVID_NONE (-1) or PLATFORM_DEVID_AUTO (-2) as id base. In this
case it does not make sense to append the cell id to the mfd-id base and
potentially change the requested behaviour.
Specifically this will allow multi-function devices to be registered
with PLATFORM_DEVID_AUTO while still having non-zero cell ids.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/mfd/mfd-core.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
index 892d343193ad..79f25633d7db 100644
--- a/drivers/mfd/mfd-core.c
+++ b/drivers/mfd/mfd-core.c
@@ -87,9 +87,15 @@ static int mfd_add_device(struct device *parent, int id,
struct platform_device *pdev;
struct device_node *np = NULL;
int ret = -ENOMEM;
+ int platform_id;
int r;
- pdev = platform_device_alloc(cell->name, id + cell->id);
+ if (id < 0)
+ platform_id = id;
+ else
+ platform_id = id + cell->id;
+
+ pdev = platform_device_alloc(cell->name, platform_id);
if (!pdev)
goto fail_alloc;
--
1.8.5.5
^ permalink raw reply related
* Re: [PATCH 0/5] HID: wacom: introduce generic HID handling
From: Jiri Kosina @ 2014-09-26 11:03 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: Ping Cheng, killertofu, linux-input, linux-kernel
In-Reply-To: <1411488489-10625-1-git-send-email-benjamin.tissoires@redhat.com>
On Tue, 23 Sep 2014, Benjamin Tissoires wrote:
> Hi guys,
>
> So, this patch series aims at supporting natively any future HID compliant wacom
> tablet. Those found on the various laptops (ISDv4/5) already are HID compliant
> and they should work in the future without any modification of the kernel.
>
> Few things to note here:
> - I used the PID 0x00E6 found on the Lenovo X230 for the tests
> - I did not removed its entry in the list because there is a slightly different
> behavior while using this patch set: when more than two fingers are on the
> screen, the tablet stops sending finger events, while with the wacom
> proprietary bits, it continues to send them. Besides that, the events emitted
> before and after the patch series are the same (at least on the E6)
> - I can not rely on hid-input directly because wacom wants to be in control of
> the input devices it creates. This might be solved in the future, but for now
> it is just easier to rewrite the few mapping/events handling than trying to
> fit in the hid-input model.
> - there will still be more specific use cases to handle later (see the joke of
> the MS surface 3 pen[1] for instance), but this should give a roughly working
> pen/touch support for those future devices.
>
> Jason, I would be very glad if you could conduct a few tests for this on the
> ISDv4/5 sensors you have.
I am waiting with this one for testing results from Jason, but if you guys
want this to go in the next merge window, I'd like to ask you not to
postpone the testing too much.
Thanks.
>
> Cheers,
> Benjamin
>
> [1] http://who-t.blogspot.com/2014/09/stylus-behaviour-on-microsoft-surface-3.html
>
> Benjamin Tissoires (5):
> HID: wacom: rename failN with some meaningful information
> HID: wacom: split out input allocation and registration
> HID: wacom: move allocation of inputs earlier
> HID: wacom: implement generic HID handling for pen generic devices
> HID: wacom: implement the finger part of the HID generic handling
>
> drivers/hid/hid-core.c | 3 +
> drivers/hid/wacom.h | 6 +
> drivers/hid/wacom_sys.c | 184 +++++++++++++++++++++--------
> drivers/hid/wacom_wac.c | 299 ++++++++++++++++++++++++++++++++++++++++++++++++
> drivers/hid/wacom_wac.h | 17 +++
> include/linux/hid.h | 2 +
> 6 files changed, 462 insertions(+), 49 deletions(-)
>
> --
> 2.1.0
>
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 5/6] HID: hid-sensor-hub: use mfd_add_hotplug_devices helper
From: Jiri Kosina @ 2014-09-26 11:25 UTC (permalink / raw)
To: Johan Hovold
Cc: Samuel Ortiz, Lee Jones, linux-input, Greg Kroah-Hartman,
linux-kernel, linux-usb
In-Reply-To: <1411728933-13351-6-git-send-email-johan@kernel.org>
On Fri, 26 Sep 2014, Johan Hovold wrote:
> Use mfd_add_hotplug_devices helper to register the subdevices.
>
> Compile-only tested.
>
> Signed-off-by: Johan Hovold <johan@kernel.org>
As I expect this to be going through MFD tree together with the rest of
the patchset
Acked-by: Jiri Kosina <jkosina@suse.cz>
> ---
> drivers/hid/hid-sensor-hub.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
> index 2ac25760a9a9..fbe1a6d2aa53 100644
> --- a/drivers/hid/hid-sensor-hub.c
> +++ b/drivers/hid/hid-sensor-hub.c
> @@ -641,9 +641,6 @@ static int sensor_hub_probe(struct hid_device *hdev,
> goto err_stop_hw;
> }
> sd->hid_sensor_hub_client_devs[
> - sd->hid_sensor_client_cnt].id =
> - PLATFORM_DEVID_AUTO;
> - sd->hid_sensor_hub_client_devs[
> sd->hid_sensor_client_cnt].name = name;
> sd->hid_sensor_hub_client_devs[
> sd->hid_sensor_client_cnt].platform_data =
> @@ -659,8 +656,9 @@ static int sensor_hub_probe(struct hid_device *hdev,
> if (last_hsdev)
> last_hsdev->end_collection_index = i;
>
> - ret = mfd_add_devices(&hdev->dev, 0, sd->hid_sensor_hub_client_devs,
> - sd->hid_sensor_client_cnt, NULL, 0, NULL);
> + ret = mfd_add_hotplug_devices(&hdev->dev,
> + sd->hid_sensor_hub_client_devs,
> + sd->hid_sensor_client_cnt);
> if (ret < 0)
> goto err_stop_hw;
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: How to indicate hover touch when exact distance unknown?
From: Benjamin Tissoires @ 2014-09-26 13:50 UTC (permalink / raw)
To: Henrik Rydberg
Cc: Peter Hutterer, Dmitry Torokhov, Andrew de los Reyes,
Hans de Goede, jikos, David Herrmann, Linux Input
In-Reply-To: <54225667.30801@euromail.se>
On Wed, Sep 24, 2014 at 1:28 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
>>>> FWIW, hid-multitouch already support those devices.
>>>> ABS_MT_DISTANCE is set with a min/max of 0/1 when we detect win8
>>>> certified panels with hovering capability. By default the spec does not
>>>> provide the distance IIRC, and you only have one byte: InRange.
>>>
>>> Hmm, I missed that and this is unfortunate. The ABS capabilities
>>> advertised by the devices should match their real capabilities. If
>>> device can't properly report distance it should not be using
>>> ABS_MT_DISTANCE/ABS_DISTANCE...
>>
>> I think the hid-mt behaviour makes sense. Without explicit resolution (and
>> no device sets that anyway, IIRC) any distance value > min tells us only that a
>> tool is within detectable range, but not yet touching. Anything between
>> min/max is only useful as a relative scale, but effectively that [min,max]
>> range could be a metre or a millimeter, we can't know. So a device with a
>> 0/1 range simply has low granularity and is only able to detect whether
>> something is within range or touching the surface.
>
> I agree with this, but I also share Dmitry's concern.
>
> A device that can detect hovering, if only binary, does in fact coarsely
> estimate the distance from the touching surface. A device allowing for a smooth
> approach of objects would simply support a better resolution. From that
> perspective, using the ABS_MT_DISTANCE capability makes sense. Pragmatically.
I agree.
>
> However, at no point are we really changing the coordinate system, which remains
> euclidian space. We are simply changing resolution and thresholds for what
> constitues a touch. Forcing userland to step away from the simple interpretation
> is what eventually makes the capability impossible to use as intended.
I think you are missing a point here. My maths are a little bit rusty,
but if euclidian space there is, the space is *not* normalized. Peter
remembered me the other day that the touchpad found on the Lenovo x220
has a x resolution of 75 and a y resolution of 129. So every sane
library/driver/tool has to take the individual resolution into account
if they want to provide accurate results.
Given that libinput, xorg-evdev and xorg-synaptics all take this into
account, then it is safe to say that they can simply consider that a
resolution of 0 is simply absolute and a binary explanation fits well
(was it 1 mm, 1 cm, 1 inch or 1 meter).
>
> So, if we cannot express, using the abs_info data, something like "contains a
> detector which can coarsely estimate the distance and then uses a detector
> threshold to set that distance to zero or one", we had better express it in some
> other way, which is less ambiguous.
IMO, your sentence is already ambiguous enough :-)
Seriously, I think that we should not worry too much about the binary
ABS_MT_DISTANCE:
Think about the wacom pens: they report hovering and distance, but I
don't think any application uses the absolute distance to change the
behavior. The user can not maintain a constant distance with the tip
of the finger or the stylus from the surface, so most of the time the
value is ignored while the hovering matters.
A binary ABS_MT_DISTANCE is enough to send this info to the driver,
and then, the driver can decide to transfer it or not to its clients.
>
> How about ABS_MT_PRESENT and/or ABS_PRESENT? It would complement TOUCH in the
> case of hovering, allow the state where the tool is there but not touching, and
> would unambiguously advertise the capability of detecting presence. It would
> also be forward compatible with additional capabilities, such as reporting the
> actual distance to the surface.
I don't think adding such a new axis is a good idea. We do *not* have
a per slot MT_TOUCH. We only have the tracking id which says that the
slot is *valid*. Then, the spec already explains how can the device
convey the hovering information:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/input/multi-touch-protocol.txt#n259
"ABS_MT_DISTANCE: The distance, in surface units, between the contact
and the surface. Zero distance means the contact is touching the
surface. A positive number means the contact is hovering above the
surface."
The problem here is the "surface units", but given that the units on
each axes depends on the per axis resolution, a resolution of 0 says
that the units are undefined, and that the client should use only the
last part of the definition: zero = touch, >0 = hovering.
my 2 cents.
Benjamin
^ permalink raw reply
* [git pull] Input updates for 3.17-rc6
From: Dmitry Torokhov @ 2014-09-26 18:01 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linux-input
Hi Linus,
Please pull from:
git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus
or
master.kernel.org:/pub/scm/linux/kernel/git/dtor/input.git for-linus
to receive updates for the input subsystem. You will get a small fixup to
i8042 adding Asus X450LCP to the nomux list.
Changelog:
---------
Marcos Paulo de Souza (1):
Input: i8042 - fix Asus X450LCP touchpad detection
Diffstat:
--------
drivers/input/serio/i8042-x86ia64io.h | 7 +++++++
1 file changed, 7 insertions(+)
--
Dmitry
^ permalink raw reply
* Re: [git pull] Input updates for 3.17-rc6
From: Linus Torvalds @ 2014-09-26 18:19 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Linux Kernel Mailing List, linux-input@vger.kernel.org
In-Reply-To: <20140926180111.GA3911@core.coreip.homeip.net>
On Fri, Sep 26, 2014 at 11:01 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> to receive updates for the input subsystem. You will get a small fixup to
> i8042 adding Asus X450LCP to the nomux list.
Side note: I'm wondering how painful it would be to just make nomux
the default.
I think it's getting to be a legacy issue, since the traditional
reason for it (afaik) is just having both an external PS/2 port _and_
a touchpad connnected to the same controller. And even when that
happens, it seems that as often as not, the bug reports in this area
have been about the touchpad _not_ working due to us incorrectly
thinking it's muxed.
I dunno. Maybe mux mode is used more than I think it is, and it would
cause tons of problems. But I do find myself wondering whether we'd be
better off not doing mux by default, and making the DMI check instead
check for systems that *do* want muxing.
Hmm? Am I just being crazy?
Linus
^ permalink raw reply
* Re: [git pull] Input updates for 3.17-rc6
From: Dmitry Torokhov @ 2014-09-26 19:14 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Linux Kernel Mailing List, linux-input@vger.kernel.org
In-Reply-To: <CA+55aFzD0Vo2c74_M-k-0Q6kGRd8btYDM34vnYA-UOP6Mko2QA@mail.gmail.com>
On Fri, Sep 26, 2014 at 11:19:51AM -0700, Linus Torvalds wrote:
> On Fri, Sep 26, 2014 at 11:01 AM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >
> > to receive updates for the input subsystem. You will get a small fixup to
> > i8042 adding Asus X450LCP to the nomux list.
>
> Side note: I'm wondering how painful it would be to just make nomux
> the default.
>
> I think it's getting to be a legacy issue, since the traditional
> reason for it (afaik) is just having both an external PS/2 port _and_
> a touchpad connnected to the same controller. And even when that
> happens, it seems that as often as not, the bug reports in this area
> have been about the touchpad _not_ working due to us incorrectly
> thinking it's muxed.
>
> I dunno. Maybe mux mode is used more than I think it is, and it would
> cause tons of problems. But I do find myself wondering whether we'd be
> better off not doing mux by default, and making the DMI check instead
> check for systems that *do* want muxing.
>
> Hmm? Am I just being crazy?
Not a all. In fact, we've been discussing it recently and I am going to flip
the flag for 3.18. I really think that number users with laptops correctly
implementing MUX and who are also using docking stations with external PS/2
mice) is miniscule (note that we are not talking about users who prefer
external mouse - you don't need MUX for that - we are talking about users who
want native touchpad protocol and also external mouse - at the same time).
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 0/5] HID: wacom: introduce generic HID handling
From: Jason Gerecke @ 2014-09-27 1:35 UTC (permalink / raw)
To: Jiri Kosina; +Cc: Benjamin Tissoires, Ping Cheng, Linux Input, linux-kernel
In-Reply-To: <alpine.LNX.2.00.1409261302520.22196@pobox.suse.cz>
On Fri, Sep 26, 2014 at 4:03 AM, Jiri Kosina <jkosina@suse.cz> wrote:
> On Tue, 23 Sep 2014, Benjamin Tissoires wrote:
>
>> Hi guys,
>>
>> So, this patch series aims at supporting natively any future HID compliant wacom
>> tablet. Those found on the various laptops (ISDv4/5) already are HID compliant
>> and they should work in the future without any modification of the kernel.
>>
>> Few things to note here:
>> - I used the PID 0x00E6 found on the Lenovo X230 for the tests
>> - I did not removed its entry in the list because there is a slightly different
>> behavior while using this patch set: when more than two fingers are on the
>> screen, the tablet stops sending finger events, while with the wacom
>> proprietary bits, it continues to send them. Besides that, the events emitted
>> before and after the patch series are the same (at least on the E6)
>> - I can not rely on hid-input directly because wacom wants to be in control of
>> the input devices it creates. This might be solved in the future, but for now
>> it is just easier to rewrite the few mapping/events handling than trying to
>> fit in the hid-input model.
>> - there will still be more specific use cases to handle later (see the joke of
>> the MS surface 3 pen[1] for instance), but this should give a roughly working
>> pen/touch support for those future devices.
>>
>> Jason, I would be very glad if you could conduct a few tests for this on the
>> ISDv4/5 sensors you have.
>
> I am waiting with this one for testing results from Jason, but if you guys
> want this to go in the next merge window, I'd like to ask you not to
> postpone the testing too much.
>
> Thanks.
>
I'm in the process of capturing traces from hid-record for multiple
tablet PCs both pre- and post-patch to locate any obvious issues. My
initial results are promising on the pen side (the only things I've
noticed so far is the 2nd barrel switch not being supported -- as
might be suspected) but less so on the touch side (single-finger
devices seem to work fine, but the new code doesn't seem to handle any
of the multitouch devices I've tried).
Expect more news next week :)
PS: I'm pretty sure ISDv5 shouldn't be relied on for HID data :| The
descriptor for the Cintiq Companion Hybrid, at least, is like our
branded sensors with useful data only in the touchscreen descriptor
(the pen descriptor is a bunch of opaque vendor-defined reports). Not
sure if the Cintiq Companion is the same though since it runs
Windows...
Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one /
(That is to say, eight) to the two, /
But you can’t take seven from three, /
So you look at the sixty-fours....
>>
>> Cheers,
>> Benjamin
>>
>> [1] http://who-t.blogspot.com/2014/09/stylus-behaviour-on-microsoft-surface-3.html
>>
>> Benjamin Tissoires (5):
>> HID: wacom: rename failN with some meaningful information
>> HID: wacom: split out input allocation and registration
>> HID: wacom: move allocation of inputs earlier
>> HID: wacom: implement generic HID handling for pen generic devices
>> HID: wacom: implement the finger part of the HID generic handling
>>
>> drivers/hid/hid-core.c | 3 +
>> drivers/hid/wacom.h | 6 +
>> drivers/hid/wacom_sys.c | 184 +++++++++++++++++++++--------
>> drivers/hid/wacom_wac.c | 299 ++++++++++++++++++++++++++++++++++++++++++++++++
>> drivers/hid/wacom_wac.h | 17 +++
>> include/linux/hid.h | 2 +
>> 6 files changed, 462 insertions(+), 49 deletions(-)
>>
>> --
>> 2.1.0
>>
>
> --
> Jiri Kosina
> SUSE Labs
^ permalink raw reply
* Elantech PS/2 touchpad stops responding after a suspend/resume in Asus X450LCP
From: Marcos Paulo de Souza @ 2014-09-27 14:41 UTC (permalink / raw)
To: linux-input, Dmitry Torokhov
Hi guys,
after putting my Asus in the nomux list, it stops to work after I close
and reopen my laptop(suspend/resume). Do you have any idea what's going on?
I tried to get some additional information in dmesg before and after it
stops working, but it seems that I don't give any useful information there.
Thanks,
Marcos
^ permalink raw reply
* Re: [PATCH v3 1/3] cap11xx: make driver generic for variant support
From: Daniel Mack @ 2014-09-27 18:21 UTC (permalink / raw)
To: Matt Ranostay, galak, dmitry.torokhov, zonque, linux-input,
linux-kernel, robh+dt
Cc: devicetree
In-Reply-To: <1411622662-16581-2-git-send-email-mranostay@gmail.com>
On 09/25/2014 07:24 AM, Matt Ranostay wrote:
> cap1106 driver can support much more one device make the driver
> generic for support of similiar parts.
>
> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
> ---
> .../devicetree/bindings/input/cap1106.txt | 53 ----
> .../devicetree/bindings/input/cap11xx.txt | 54 ++++
> drivers/input/keyboard/Kconfig | 8 +-
> drivers/input/keyboard/Makefile | 2 +-
> drivers/input/keyboard/cap1106.c | 341 ---------------------
> drivers/input/keyboard/cap11xx.c | 340 ++++++++++++++++++++
> 6 files changed, 399 insertions(+), 399 deletions(-)
As I've said, such a patch is really a whole lot more readable if you
use the -M switch for git format-patch to detect renames.
Other than that, the series now looks good to me. I can't test it,
however, as I don't have access to the hardware anymore.
Reviewed-by: Daniel Mack <daniel@zonque.org>
Thanks,
Daniel
^ permalink raw reply
* [PATCH RFC] drivers: parport: Ask user for irqreturn_t value
From: Matwey V. Kornilov @ 2014-09-28 8:02 UTC (permalink / raw)
To: gregkh
Cc: linux-parport, arnd, jdelvare, linux-i2c, dmitry.torokhov,
linux-input, t.sailer, linux-hams, netdev, giometti, linuxpps,
m.chehab, linux-media, linux-kernel, Matwey V. Kornilov
Current parport_irq_handler behaviour is not correct when IRQ is shared.
LDDv3 on page 279 requires us:
"Be sure to return IRQ_NONE whenever your handler is called and finds
the device is not interrupting."
This is not the case of parport_irq_handler.
Current implementation of IRQ handling use this to optimize shared IRQ processing.
When an action returns IRQ_HANDLED no other actions are processed.
Modify callback function void (*irq_func)(void *) to irqreturn_t (*irq_func)(void *) and return the value in parport_irq_handler
This also fixes https://bugzilla.kernel.org/show_bug.cgi?id=85221
Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru>
---
drivers/char/ppdev.c | 4 +++-
drivers/i2c/busses/i2c-parport.c | 5 ++++-
drivers/input/joystick/walkera0701.c | 6 ++++--
drivers/net/hamradio/baycom_par.c | 4 +++-
drivers/net/plip/plip.c | 8 +++++---
drivers/parport/share.c | 6 ++----
drivers/pps/clients/pps_parport.c | 6 +++---
drivers/staging/media/lirc/lirc_parallel.c | 12 +++++++-----
include/linux/parport.h | 12 ++++++++----
9 files changed, 39 insertions(+), 24 deletions(-)
diff --git a/drivers/char/ppdev.c b/drivers/char/ppdev.c
index ae0b42b..61fec51 100644
--- a/drivers/char/ppdev.c
+++ b/drivers/char/ppdev.c
@@ -264,7 +264,7 @@ static ssize_t pp_write (struct file * file, const char __user * buf,
return bytes_written;
}
-static void pp_irq (void *private)
+static irqreturn_t pp_irq (void *private)
{
struct pp_struct *pp = private;
@@ -275,6 +275,8 @@ static void pp_irq (void *private)
atomic_inc (&pp->irqc);
wake_up_interruptible (&pp->irq_wait);
+
+ return IRQ_HANDLED;
}
static int register_device (int minor, struct pp_struct *pp)
diff --git a/drivers/i2c/busses/i2c-parport.c b/drivers/i2c/busses/i2c-parport.c
index a27aae2..7549e10 100644
--- a/drivers/i2c/busses/i2c-parport.c
+++ b/drivers/i2c/busses/i2c-parport.c
@@ -151,7 +151,7 @@ static const struct i2c_algo_bit_data parport_algo_data = {
/* ----- I2c and parallel port call-back functions and structures --------- */
-static void i2c_parport_irq(void *data)
+static irqreturn_t i2c_parport_irq(void *data)
{
struct i2c_par *adapter = data;
struct i2c_client *ara = adapter->ara;
@@ -159,9 +159,12 @@ static void i2c_parport_irq(void *data)
if (ara) {
dev_dbg(&ara->dev, "SMBus alert received\n");
i2c_handle_smbus_alert(ara);
+ return IRQ_HANDLED;
} else
dev_dbg(&adapter->adapter.dev,
"SMBus alert received but no ARA client!\n");
+
+ return IRQ_NONE;
}
static void i2c_parport_attach(struct parport *port)
diff --git a/drivers/input/joystick/walkera0701.c b/drivers/input/joystick/walkera0701.c
index b76ac58..f3903a6 100644
--- a/drivers/input/joystick/walkera0701.c
+++ b/drivers/input/joystick/walkera0701.c
@@ -124,7 +124,7 @@ static inline int read_ack(struct pardevice *p)
}
/* falling edge, prepare to BIN value calculation */
-static void walkera0701_irq_handler(void *handler_data)
+static irqreturn_t walkera0701_irq_handler(void *handler_data)
{
u64 pulse_time;
struct walkera_dev *w = handler_data;
@@ -136,7 +136,7 @@ static void walkera0701_irq_handler(void *handler_data)
/* cancel timer, if in handler or active do resync */
if (unlikely(0 != hrtimer_try_to_cancel(&w->timer))) {
w->counter = NO_SYNC;
- return;
+ return IRQ_HANDLED;
}
if (w->counter < NO_SYNC) {
@@ -166,6 +166,8 @@ static void walkera0701_irq_handler(void *handler_data)
w->counter = 0;
hrtimer_start(&w->timer, ktime_set(0, BIN_SAMPLE), HRTIMER_MODE_REL);
+
+ return IRQ_HANDLED;
}
static enum hrtimer_restart timer_handler(struct hrtimer
diff --git a/drivers/net/hamradio/baycom_par.c b/drivers/net/hamradio/baycom_par.c
index acb6369..1fd9a7b 100644
--- a/drivers/net/hamradio/baycom_par.c
+++ b/drivers/net/hamradio/baycom_par.c
@@ -269,7 +269,7 @@ static __inline__ void par96_rx(struct net_device *dev, struct baycom_state *bc)
/* --------------------------------------------------------------------- */
-static void par96_interrupt(void *dev_id)
+static irqreturn_t par96_interrupt(void *dev_id)
{
struct net_device *dev = dev_id;
struct baycom_state *bc = netdev_priv(dev);
@@ -292,6 +292,8 @@ static void par96_interrupt(void *dev_id)
hdlcdrv_transmitter(dev, &bc->hdrv);
hdlcdrv_receiver(dev, &bc->hdrv);
local_irq_disable();
+
+ return IRQ_HANDLED;
}
/* --------------------------------------------------------------------- */
diff --git a/drivers/net/plip/plip.c b/drivers/net/plip/plip.c
index 040b897..00a8b3a 100644
--- a/drivers/net/plip/plip.c
+++ b/drivers/net/plip/plip.c
@@ -143,7 +143,7 @@ static void plip_bh(struct work_struct *work);
static void plip_timer_bh(struct work_struct *work);
/* Interrupt handler */
-static void plip_interrupt(void *dev_id);
+static irqreturn_t plip_interrupt(void *dev_id);
/* Functions for DEV methods */
static int plip_tx_packet(struct sk_buff *skb, struct net_device *dev);
@@ -900,7 +900,7 @@ plip_error(struct net_device *dev, struct net_local *nl,
}
/* Handle the parallel port interrupts. */
-static void
+static irqreturn_t
plip_interrupt(void *dev_id)
{
struct net_device *dev = dev_id;
@@ -919,7 +919,7 @@ plip_interrupt(void *dev_id)
if ((dev->irq != -1) && (net_debug > 1))
printk(KERN_DEBUG "%s: spurious interrupt\n", dev->name);
spin_unlock_irqrestore (&nl->lock, flags);
- return;
+ return IRQ_NONE;
}
if (net_debug > 3)
@@ -948,6 +948,8 @@ plip_interrupt(void *dev_id)
}
spin_unlock_irqrestore(&nl->lock, flags);
+
+ return IRQ_HANDLED;
}
static int
diff --git a/drivers/parport/share.c b/drivers/parport/share.c
index 3fa6624..1fd8dc2 100644
--- a/drivers/parport/share.c
+++ b/drivers/parport/share.c
@@ -523,7 +523,7 @@ void parport_remove_port(struct parport *port)
struct pardevice *
parport_register_device(struct parport *port, const char *name,
int (*pf)(void *), void (*kf)(void *),
- void (*irq_func)(void *),
+ irqreturn_t (*irq_func)(void *),
int flags, void *handle)
{
struct pardevice *tmp;
@@ -1006,9 +1006,7 @@ irqreturn_t parport_irq_handler(int irq, void *dev_id)
{
struct parport *port = dev_id;
- parport_generic_irq(port);
-
- return IRQ_HANDLED;
+ return parport_generic_irq(port);
}
/* Exported symbols for modules. */
diff --git a/drivers/pps/clients/pps_parport.c b/drivers/pps/clients/pps_parport.c
index 38a8bbe..9acf376 100644
--- a/drivers/pps/clients/pps_parport.c
+++ b/drivers/pps/clients/pps_parport.c
@@ -64,7 +64,7 @@ static inline int signal_is_set(struct parport *port)
}
/* parport interrupt handler */
-static void parport_irq(void *handle)
+static irqreturn_t parport_irq(void *handle)
{
struct pps_event_time ts_assert, ts_clear;
struct pps_client_pp *dev = handle;
@@ -122,7 +122,7 @@ out_assert:
/* fire assert event */
pps_event(dev->pps, &ts_assert,
PPS_CAPTUREASSERT, NULL);
- return;
+ return IRQ_HANDLED;
out_both:
/* fire assert event */
@@ -131,7 +131,7 @@ out_both:
/* fire clear event */
pps_event(dev->pps, &ts_clear,
PPS_CAPTURECLEAR, NULL);
- return;
+ return IRQ_HANDLED;
}
static void parport_attach(struct parport *port)
diff --git a/drivers/staging/media/lirc/lirc_parallel.c b/drivers/staging/media/lirc/lirc_parallel.c
index 672858a..63fdc27 100644
--- a/drivers/staging/media/lirc/lirc_parallel.c
+++ b/drivers/staging/media/lirc/lirc_parallel.c
@@ -219,7 +219,7 @@ static void rbuf_write(int signal)
wptr = nwptr;
}
-static void lirc_lirc_irq_handler(void *blah)
+static irqreturn_t lirc_lirc_irq_handler(void *blah)
{
struct timeval tv;
static struct timeval lasttv;
@@ -230,10 +230,10 @@ static void lirc_lirc_irq_handler(void *blah)
unsigned int timeout;
if (!is_open)
- return;
+ return IRQ_NONE;
if (!is_claimed)
- return;
+ return IRQ_NONE;
#if 0
/* disable interrupt */
@@ -241,7 +241,7 @@ static void lirc_lirc_irq_handler(void *blah)
out(LIRC_PORT_IRQ, in(LIRC_PORT_IRQ) & (~LP_PINTEN));
#endif
if (check_pselecd && (in(1) & LP_PSELECD))
- return;
+ return IRQ_NONE;
#ifdef LIRC_TIMER
if (init) {
@@ -265,7 +265,7 @@ static void lirc_lirc_irq_handler(void *blah)
*/
timer = init_lirc_timer();
/* enable_irq(irq); */
- return;
+ return IRQ_HANDLED;
}
init = 1;
}
@@ -314,6 +314,8 @@ static void lirc_lirc_irq_handler(void *blah)
enable_irq(irq);
out(LIRC_PORT_IRQ, in(LIRC_PORT_IRQ)|LP_PINTEN);
*/
+
+ return IRQ_HANDLED;
}
/*** file operations ***/
diff --git a/include/linux/parport.h b/include/linux/parport.h
index c22f125..2ab774c 100644
--- a/include/linux/parport.h
+++ b/include/linux/parport.h
@@ -141,7 +141,7 @@ struct pardevice {
int (*preempt)(void *);
void (*wakeup)(void *);
void *private;
- void (*irq_func)(void *);
+ irqreturn_t (*irq_func)(void *);
unsigned int flags;
struct pardevice *next;
struct pardevice *prev;
@@ -298,7 +298,7 @@ extern void parport_put_port (struct parport *);
struct pardevice *parport_register_device(struct parport *port,
const char *name,
int (*pf)(void *), void (*kf)(void *),
- void (*irq_func)(void *),
+ irqreturn_t (*irq_func)(void *),
int flags, void *handle);
/* parport_unregister unlinks a device from the chain. */
@@ -429,13 +429,17 @@ extern void parport_daisy_deselect_all (struct parport *port);
extern int parport_daisy_select (struct parport *port, int daisy, int mode);
/* Lowlevel drivers _can_ call this support function to handle irqs. */
-static inline void parport_generic_irq(struct parport *port)
+static inline irqreturn_t parport_generic_irq(struct parport *port)
{
+ irqreturn_t ret = IRQ_NONE;
+
parport_ieee1284_interrupt (port);
read_lock(&port->cad_lock);
if (port->cad && port->cad->irq_func)
- port->cad->irq_func(port->cad->private);
+ ret = port->cad->irq_func(port->cad->private);
read_unlock(&port->cad_lock);
+
+ return ret;
}
/* Prototypes from parport_procfs */
--
2.1.0
^ permalink raw reply related
* Re: [PATCH RESEND RESEND] Input: evdev - add event-mask API
From: David Herrmann @ 2014-09-28 10:19 UTC (permalink / raw)
To: open list:HID CORE LAYER, Dmitry Torokhov
Cc: Peter Hutterer, Benjamin Tissoires, David Herrmann
In-Reply-To: <1407914195-2797-1-git-send-email-dh.herrmann@gmail.com>
Ping?
On Wed, Aug 13, 2014 at 9:16 AM, David Herrmann <dh.herrmann@gmail.com> wrote:
> Hardware manufacturers group keys in the weirdest way possible. This may
> cause a power-key to be grouped together with normal keyboard keys and
> thus be reported on the same kernel interface.
>
> However, user-space is often only interested in specific sets of events.
> For instance, daemons dealing with system-reboot (like systemd-logind)
> listen for KEY_POWER, but are not interested in any main keyboard keys.
> Usually, power keys are reported via separate interfaces, however,
> some i8042 boards report it in the AT matrix. To avoid waking up those
> system daemons on each key-press, we had two ideas:
> - split off KEY_POWER into a separate interface unconditionally
> - allow filtering a specific set of events on evdev FDs
>
> Splitting of KEY_POWER is a rather weird way to deal with this and may
> break backwards-compatibility. It is also specific to KEY_POWER and might
> be required for other stuff, too. Moreover, we might end up with a huge
> set of input-devices just to have them properly split.
>
> Hence, this patchset implements the second idea: An event-mask to specify
> which events you're interested in. Two ioctls allow setting this mask for
> each event-type. If not set, all events are reported. The type==0 entry is
> used same as in EVIOCGBIT to set the actual EV_* mask of filtered events.
> This way, you have a two-level filter.
>
> We are heavily forward-compatible to new event-types and event-codes. So
> new user-space will be able to run on an old kernel which doesn't know the
> given event-codes or event-types.
>
> Acked-by: Peter Hutterer <peter.hutterer@who-t.net>
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> ---
> Hi Dmitry
>
> We could really make use of this for SUSPEND/POWER key handling. We keep getting
> reports from people where those keys are reported as part of the main keyboard.
> It's really annoying if we have to wake up those processes for *every*
> key-press.
>
> In case you just need time to review it, let me know. Otherwise, I will keep
> resending it as people ask me for it all the time.
>
> Thanks
> David
>
> drivers/input/evdev.c | 156 ++++++++++++++++++++++++++++++++++++++++++++-
> include/uapi/linux/input.h | 56 ++++++++++++++++
> 2 files changed, 210 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> index fd325ec..6386882 100644
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c
> @@ -51,10 +51,130 @@ struct evdev_client {
> struct list_head node;
> int clkid;
> bool revoked;
> + unsigned long *evmasks[EV_CNT];
> unsigned int bufsize;
> struct input_event buffer[];
> };
>
> +static size_t evdev_get_mask_cnt(unsigned int type)
> +{
> + static size_t counts[EV_CNT] = {
> + /* EV_SYN==0 is EV_CNT, _not_ SYN_CNT, see EVIOCGBIT */
> + [EV_SYN] = EV_CNT,
> + [EV_KEY] = KEY_CNT,
> + [EV_REL] = REL_CNT,
> + [EV_ABS] = ABS_CNT,
> + [EV_MSC] = MSC_CNT,
> + [EV_SW] = SW_CNT,
> + [EV_LED] = LED_CNT,
> + [EV_SND] = SND_CNT,
> + [EV_FF] = FF_CNT,
> + };
> +
> + return (type < EV_CNT) ? counts[type] : 0;
> +}
> +
> +/* must be called with evdev-mutex held */
> +static int evdev_set_mask(struct evdev_client *client,
> + unsigned int type,
> + const void __user *codes,
> + u32 codes_size)
> +{
> + unsigned long flags, *mask, *oldmask;
> + size_t cnt, size;
> +
> + /* unknown masks are simply ignored for forward-compat */
> + cnt = evdev_get_mask_cnt(type);
> + if (!cnt)
> + return 0;
> +
> + /* we allow 'codes_size > size' for forward-compat */
> + size = sizeof(unsigned long) * BITS_TO_LONGS(cnt);
> +
> + mask = kzalloc(size, GFP_KERNEL);
> + if (!mask)
> + return -ENOMEM;
> +
> + if (copy_from_user(mask, codes, min_t(size_t, codes_size, size))) {
> + kfree(mask);
> + return -EFAULT;
> + }
> +
> + spin_lock_irqsave(&client->buffer_lock, flags);
> + oldmask = client->evmasks[type];
> + client->evmasks[type] = mask;
> + spin_unlock_irqrestore(&client->buffer_lock, flags);
> +
> + kfree(oldmask);
> +
> + return 0;
> +}
> +
> +/* must be called with evdev-mutex held */
> +static int evdev_get_mask(struct evdev_client *client,
> + unsigned int type,
> + void __user *codes,
> + u32 codes_size)
> +{
> + unsigned long *mask;
> + size_t cnt, size, min, i;
> + u8 __user *out;
> +
> + /* we allow unknown types and 'codes_size > size' for forward-compat */
> + cnt = evdev_get_mask_cnt(type);
> + size = sizeof(unsigned long) * BITS_TO_LONGS(cnt);
> + min = min_t(size_t, codes_size, size);
> +
> + if (cnt > 0) {
> + mask = client->evmasks[type];
> + if (mask) {
> + if (copy_to_user(codes, mask, min))
> + return -EFAULT;
> + } else {
> + /* fake mask with all bits set */
> + out = (u8 __user*)codes;
> + for (i = 0; i < min; ++i) {
> + if (put_user((u8)0xff, out + i))
> + return -EFAULT;
> + }
> + }
> + }
> +
> + codes = (u8*)codes + min;
> + codes_size -= min;
> +
> + if (codes_size > 0 && clear_user(codes, codes_size))
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> +/* requires the buffer lock to be held */
> +static bool __evdev_is_filtered(struct evdev_client *client,
> + unsigned int type,
> + unsigned int code)
> +{
> + unsigned long *mask;
> + size_t cnt;
> +
> + /* EV_SYN and unknown codes are never filtered */
> + if (type == EV_SYN || type >= EV_CNT)
> + return false;
> +
> + /* first test whether the type is filtered */
> + mask = client->evmasks[0];
> + if (mask && !test_bit(type, mask))
> + return true;
> +
> + /* unknown values are never filtered */
> + cnt = evdev_get_mask_cnt(type);
> + if (!cnt || code >= cnt)
> + return false;
> +
> + mask = client->evmasks[type];
> + return mask && !test_bit(code, mask);
> +}
> +
> /* flush queued events of type @type, caller must hold client->buffer_lock */
> static void __evdev_flush_queue(struct evdev_client *client, unsigned int type)
> {
> @@ -177,12 +297,21 @@ static void evdev_pass_values(struct evdev_client *client,
> spin_lock(&client->buffer_lock);
>
> for (v = vals; v != vals + count; v++) {
> + if (__evdev_is_filtered(client, v->type, v->code))
> + continue;
> +
> + if (v->type == EV_SYN && v->code == SYN_REPORT) {
> + /* drop empty SYN_REPORT */
> + if (client->packet_head == client->head)
> + continue;
> +
> + wakeup = true;
> + }
> +
> event.type = v->type;
> event.code = v->code;
> event.value = v->value;
> __pass_event(client, &event);
> - if (v->type == EV_SYN && v->code == SYN_REPORT)
> - wakeup = true;
> }
>
> spin_unlock(&client->buffer_lock);
> @@ -365,6 +494,7 @@ static int evdev_release(struct inode *inode, struct file *file)
> {
> struct evdev_client *client = file->private_data;
> struct evdev *evdev = client->evdev;
> + unsigned int i;
>
> mutex_lock(&evdev->mutex);
> evdev_ungrab(evdev, client);
> @@ -372,6 +502,9 @@ static int evdev_release(struct inode *inode, struct file *file)
>
> evdev_detach_client(evdev, client);
>
> + for (i = 0; i < EV_CNT; ++i)
> + kfree(client->evmasks[i]);
> +
> if (is_vmalloc_addr(client))
> vfree(client);
> else
> @@ -811,6 +944,7 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
> struct evdev *evdev = client->evdev;
> struct input_dev *dev = evdev->handle.dev;
> struct input_absinfo abs;
> + struct input_mask mask;
> struct ff_effect effect;
> int __user *ip = (int __user *)p;
> unsigned int i, t, u, v;
> @@ -872,6 +1006,24 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
> else
> return evdev_revoke(evdev, client, file);
>
> + case EVIOCGMASK:
> + if (copy_from_user(&mask, p, sizeof(mask)))
> + return -EFAULT;
> +
> + return evdev_get_mask(client,
> + mask.type,
> + (void*)(long)mask.codes_ptr,
> + mask.codes_size);
> +
> + case EVIOCSMASK:
> + if (copy_from_user(&mask, p, sizeof(mask)))
> + return -EFAULT;
> +
> + return evdev_set_mask(client,
> + mask.type,
> + (const void*)(long)mask.codes_ptr,
> + mask.codes_size);
> +
> case EVIOCSCLOCKID:
> if (copy_from_user(&i, p, sizeof(unsigned int)))
> return -EFAULT;
> diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
> index 19df18c..f6ace0e 100644
> --- a/include/uapi/linux/input.h
> +++ b/include/uapi/linux/input.h
> @@ -97,6 +97,12 @@ struct input_keymap_entry {
> __u8 scancode[32];
> };
>
> +struct input_mask {
> + __u32 type;
> + __u32 codes_size;
> + __u64 codes_ptr;
> +};
> +
> #define EVIOCGVERSION _IOR('E', 0x01, int) /* get driver version */
> #define EVIOCGID _IOR('E', 0x02, struct input_id) /* get device ID */
> #define EVIOCGREP _IOR('E', 0x03, unsigned int[2]) /* get repeat settings */
> @@ -154,6 +160,56 @@ struct input_keymap_entry {
> #define EVIOCGRAB _IOW('E', 0x90, int) /* Grab/Release device */
> #define EVIOCREVOKE _IOW('E', 0x91, int) /* Revoke device access */
>
> +/**
> + * EVIOCGMASK - Retrieve current event-mask
> + *
> + * This retrieves the current event-mask for a specific event-type. The
> + * argument must be of type "struct input_mask" and specifies the event-type to
> + * query, the receive buffer and the size of the receive buffer.
> + *
> + * The event-mask is a per-client mask that specifies which events are forwarded
> + * to the client. Each event-code is represented by a single bit in the
> + * event-mask. If the bit is set, the event is passed to the client normally.
> + * Otherwise, the event is filtered and and will never be queued on the
> + * client's receive buffer.
> + * Event-masks do not affect global state of an input-device. They only affect
> + * the open-file they're applied on. Each open-file (i.e, file-description) can
> + * have a different event-mask.
> + *
> + * The default event-mask for a client has all bits set, i.e. all events are
> + * forwarded to the client. If a kernel is queried for an unknown event-type
> + * or if the receive buffer is larger than the number of event-codes known to
> + * the kernel, the kernel returns all zeroes for those codes.
> + *
> + * At maximum, codes_size bytes are copied.
> + *
> + * This ioctl may fail with ENODEV in case the file is revoked, EFAULT
> + * if the receive-buffer points to invalid memory, or EINVAL if the kernel
> + * does not implement the ioctl.
> + */
> +#define EVIOCGMASK _IOR('E', 0x92, struct input_mask) /* Get event-masks */
> +
> +/**
> + * EVIOCSMASK - Set event-mask
> + *
> + * This is the counterpart to EVIOCGMASK. Instead of receiving the current
> + * event-mask, this changes the client's event-mask for a specific type. See
> + * EVIOCGMASK for a description of event-masks and the argument-type.
> + *
> + * This ioctl provides full forward-compatibility. If the passed event-type is
> + * unknown to the kernel, or if the number of codes is bigger than known to the
> + * kernel, the ioctl is still accepted and applied. However, any unknown codes
> + * are left untouched and stay cleared. That means, the kernel always filters
> + * unknown codes regardless of what the client requests.
> + * If the new mask doesn't cover all known event-codes, all remaining codes are
> + * automatically cleared and thus filtered.
> + *
> + * This ioctl may fail with ENODEV in case the file is revoked. EFAULT is
> + * returned if the receive-buffer points to invalid memory. EINVAL is returned
> + * if the kernel does not implement the ioctl.
> + */
> +#define EVIOCSMASK _IOW('E', 0x93, struct input_mask) /* Set event-masks */
> +
> #define EVIOCSCLOCKID _IOW('E', 0xa0, int) /* Set clockid to be used for timestamps */
>
> /*
> --
> 2.0.4
>
^ permalink raw reply
* Re: [PATCH RFC] drivers: parport: Ask user for irqreturn_t value
From: Matwey V. Kornilov @ 2014-09-28 14:30 UTC (permalink / raw)
To: Greg KH
Cc: linux-parport, arnd, Jean Delvare, linux-i2c, dmitry.torokhov,
linux-input, t.sailer, linux-hams, netdev, giometti, linuxpps,
m.chehab, linux-media, linux-kernel, Matwey V. Kornilov
In-Reply-To: <1411891335-3677-1-git-send-email-matwey-TTlVxmypnbovJsYlp49lxw@public.gmane.org>
Forget it, I invented something strange.
2014-09-28 12:02 GMT+04:00 Matwey V. Kornilov <matwey-TTlVxmypnbovJsYlp49lxw@public.gmane.org>:
> Current parport_irq_handler behaviour is not correct when IRQ is shared.
> LDDv3 on page 279 requires us:
>
> "Be sure to return IRQ_NONE whenever your handler is called and finds
> the device is not interrupting."
>
> This is not the case of parport_irq_handler.
> Current implementation of IRQ handling use this to optimize shared IRQ processing.
> When an action returns IRQ_HANDLED no other actions are processed.
>
> Modify callback function void (*irq_func)(void *) to irqreturn_t (*irq_func)(void *) and return the value in parport_irq_handler
>
> This also fixes https://bugzilla.kernel.org/show_bug.cgi?id=85221
>
> Signed-off-by: Matwey V. Kornilov <matwey-TTlVxmypnbovJsYlp49lxw@public.gmane.org>
> ---
> drivers/char/ppdev.c | 4 +++-
> drivers/i2c/busses/i2c-parport.c | 5 ++++-
> drivers/input/joystick/walkera0701.c | 6 ++++--
> drivers/net/hamradio/baycom_par.c | 4 +++-
> drivers/net/plip/plip.c | 8 +++++---
> drivers/parport/share.c | 6 ++----
> drivers/pps/clients/pps_parport.c | 6 +++---
> drivers/staging/media/lirc/lirc_parallel.c | 12 +++++++-----
> include/linux/parport.h | 12 ++++++++----
> 9 files changed, 39 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/char/ppdev.c b/drivers/char/ppdev.c
> index ae0b42b..61fec51 100644
> --- a/drivers/char/ppdev.c
> +++ b/drivers/char/ppdev.c
> @@ -264,7 +264,7 @@ static ssize_t pp_write (struct file * file, const char __user * buf,
> return bytes_written;
> }
>
> -static void pp_irq (void *private)
> +static irqreturn_t pp_irq (void *private)
> {
> struct pp_struct *pp = private;
>
> @@ -275,6 +275,8 @@ static void pp_irq (void *private)
>
> atomic_inc (&pp->irqc);
> wake_up_interruptible (&pp->irq_wait);
> +
> + return IRQ_HANDLED;
> }
>
> static int register_device (int minor, struct pp_struct *pp)
> diff --git a/drivers/i2c/busses/i2c-parport.c b/drivers/i2c/busses/i2c-parport.c
> index a27aae2..7549e10 100644
> --- a/drivers/i2c/busses/i2c-parport.c
> +++ b/drivers/i2c/busses/i2c-parport.c
> @@ -151,7 +151,7 @@ static const struct i2c_algo_bit_data parport_algo_data = {
>
> /* ----- I2c and parallel port call-back functions and structures --------- */
>
> -static void i2c_parport_irq(void *data)
> +static irqreturn_t i2c_parport_irq(void *data)
> {
> struct i2c_par *adapter = data;
> struct i2c_client *ara = adapter->ara;
> @@ -159,9 +159,12 @@ static void i2c_parport_irq(void *data)
> if (ara) {
> dev_dbg(&ara->dev, "SMBus alert received\n");
> i2c_handle_smbus_alert(ara);
> + return IRQ_HANDLED;
> } else
> dev_dbg(&adapter->adapter.dev,
> "SMBus alert received but no ARA client!\n");
> +
> + return IRQ_NONE;
> }
>
> static void i2c_parport_attach(struct parport *port)
> diff --git a/drivers/input/joystick/walkera0701.c b/drivers/input/joystick/walkera0701.c
> index b76ac58..f3903a6 100644
> --- a/drivers/input/joystick/walkera0701.c
> +++ b/drivers/input/joystick/walkera0701.c
> @@ -124,7 +124,7 @@ static inline int read_ack(struct pardevice *p)
> }
>
> /* falling edge, prepare to BIN value calculation */
> -static void walkera0701_irq_handler(void *handler_data)
> +static irqreturn_t walkera0701_irq_handler(void *handler_data)
> {
> u64 pulse_time;
> struct walkera_dev *w = handler_data;
> @@ -136,7 +136,7 @@ static void walkera0701_irq_handler(void *handler_data)
> /* cancel timer, if in handler or active do resync */
> if (unlikely(0 != hrtimer_try_to_cancel(&w->timer))) {
> w->counter = NO_SYNC;
> - return;
> + return IRQ_HANDLED;
> }
>
> if (w->counter < NO_SYNC) {
> @@ -166,6 +166,8 @@ static void walkera0701_irq_handler(void *handler_data)
> w->counter = 0;
>
> hrtimer_start(&w->timer, ktime_set(0, BIN_SAMPLE), HRTIMER_MODE_REL);
> +
> + return IRQ_HANDLED;
> }
>
> static enum hrtimer_restart timer_handler(struct hrtimer
> diff --git a/drivers/net/hamradio/baycom_par.c b/drivers/net/hamradio/baycom_par.c
> index acb6369..1fd9a7b 100644
> --- a/drivers/net/hamradio/baycom_par.c
> +++ b/drivers/net/hamradio/baycom_par.c
> @@ -269,7 +269,7 @@ static __inline__ void par96_rx(struct net_device *dev, struct baycom_state *bc)
>
> /* --------------------------------------------------------------------- */
>
> -static void par96_interrupt(void *dev_id)
> +static irqreturn_t par96_interrupt(void *dev_id)
> {
> struct net_device *dev = dev_id;
> struct baycom_state *bc = netdev_priv(dev);
> @@ -292,6 +292,8 @@ static void par96_interrupt(void *dev_id)
> hdlcdrv_transmitter(dev, &bc->hdrv);
> hdlcdrv_receiver(dev, &bc->hdrv);
> local_irq_disable();
> +
> + return IRQ_HANDLED;
> }
>
> /* --------------------------------------------------------------------- */
> diff --git a/drivers/net/plip/plip.c b/drivers/net/plip/plip.c
> index 040b897..00a8b3a 100644
> --- a/drivers/net/plip/plip.c
> +++ b/drivers/net/plip/plip.c
> @@ -143,7 +143,7 @@ static void plip_bh(struct work_struct *work);
> static void plip_timer_bh(struct work_struct *work);
>
> /* Interrupt handler */
> -static void plip_interrupt(void *dev_id);
> +static irqreturn_t plip_interrupt(void *dev_id);
>
> /* Functions for DEV methods */
> static int plip_tx_packet(struct sk_buff *skb, struct net_device *dev);
> @@ -900,7 +900,7 @@ plip_error(struct net_device *dev, struct net_local *nl,
> }
>
> /* Handle the parallel port interrupts. */
> -static void
> +static irqreturn_t
> plip_interrupt(void *dev_id)
> {
> struct net_device *dev = dev_id;
> @@ -919,7 +919,7 @@ plip_interrupt(void *dev_id)
> if ((dev->irq != -1) && (net_debug > 1))
> printk(KERN_DEBUG "%s: spurious interrupt\n", dev->name);
> spin_unlock_irqrestore (&nl->lock, flags);
> - return;
> + return IRQ_NONE;
> }
>
> if (net_debug > 3)
> @@ -948,6 +948,8 @@ plip_interrupt(void *dev_id)
> }
>
> spin_unlock_irqrestore(&nl->lock, flags);
> +
> + return IRQ_HANDLED;
> }
>
> static int
> diff --git a/drivers/parport/share.c b/drivers/parport/share.c
> index 3fa6624..1fd8dc2 100644
> --- a/drivers/parport/share.c
> +++ b/drivers/parport/share.c
> @@ -523,7 +523,7 @@ void parport_remove_port(struct parport *port)
> struct pardevice *
> parport_register_device(struct parport *port, const char *name,
> int (*pf)(void *), void (*kf)(void *),
> - void (*irq_func)(void *),
> + irqreturn_t (*irq_func)(void *),
> int flags, void *handle)
> {
> struct pardevice *tmp;
> @@ -1006,9 +1006,7 @@ irqreturn_t parport_irq_handler(int irq, void *dev_id)
> {
> struct parport *port = dev_id;
>
> - parport_generic_irq(port);
> -
> - return IRQ_HANDLED;
> + return parport_generic_irq(port);
> }
>
> /* Exported symbols for modules. */
> diff --git a/drivers/pps/clients/pps_parport.c b/drivers/pps/clients/pps_parport.c
> index 38a8bbe..9acf376 100644
> --- a/drivers/pps/clients/pps_parport.c
> +++ b/drivers/pps/clients/pps_parport.c
> @@ -64,7 +64,7 @@ static inline int signal_is_set(struct parport *port)
> }
>
> /* parport interrupt handler */
> -static void parport_irq(void *handle)
> +static irqreturn_t parport_irq(void *handle)
> {
> struct pps_event_time ts_assert, ts_clear;
> struct pps_client_pp *dev = handle;
> @@ -122,7 +122,7 @@ out_assert:
> /* fire assert event */
> pps_event(dev->pps, &ts_assert,
> PPS_CAPTUREASSERT, NULL);
> - return;
> + return IRQ_HANDLED;
>
> out_both:
> /* fire assert event */
> @@ -131,7 +131,7 @@ out_both:
> /* fire clear event */
> pps_event(dev->pps, &ts_clear,
> PPS_CAPTURECLEAR, NULL);
> - return;
> + return IRQ_HANDLED;
> }
>
> static void parport_attach(struct parport *port)
> diff --git a/drivers/staging/media/lirc/lirc_parallel.c b/drivers/staging/media/lirc/lirc_parallel.c
> index 672858a..63fdc27 100644
> --- a/drivers/staging/media/lirc/lirc_parallel.c
> +++ b/drivers/staging/media/lirc/lirc_parallel.c
> @@ -219,7 +219,7 @@ static void rbuf_write(int signal)
> wptr = nwptr;
> }
>
> -static void lirc_lirc_irq_handler(void *blah)
> +static irqreturn_t lirc_lirc_irq_handler(void *blah)
> {
> struct timeval tv;
> static struct timeval lasttv;
> @@ -230,10 +230,10 @@ static void lirc_lirc_irq_handler(void *blah)
> unsigned int timeout;
>
> if (!is_open)
> - return;
> + return IRQ_NONE;
>
> if (!is_claimed)
> - return;
> + return IRQ_NONE;
>
> #if 0
> /* disable interrupt */
> @@ -241,7 +241,7 @@ static void lirc_lirc_irq_handler(void *blah)
> out(LIRC_PORT_IRQ, in(LIRC_PORT_IRQ) & (~LP_PINTEN));
> #endif
> if (check_pselecd && (in(1) & LP_PSELECD))
> - return;
> + return IRQ_NONE;
>
> #ifdef LIRC_TIMER
> if (init) {
> @@ -265,7 +265,7 @@ static void lirc_lirc_irq_handler(void *blah)
> */
> timer = init_lirc_timer();
> /* enable_irq(irq); */
> - return;
> + return IRQ_HANDLED;
> }
> init = 1;
> }
> @@ -314,6 +314,8 @@ static void lirc_lirc_irq_handler(void *blah)
> enable_irq(irq);
> out(LIRC_PORT_IRQ, in(LIRC_PORT_IRQ)|LP_PINTEN);
> */
> +
> + return IRQ_HANDLED;
> }
>
> /*** file operations ***/
> diff --git a/include/linux/parport.h b/include/linux/parport.h
> index c22f125..2ab774c 100644
> --- a/include/linux/parport.h
> +++ b/include/linux/parport.h
> @@ -141,7 +141,7 @@ struct pardevice {
> int (*preempt)(void *);
> void (*wakeup)(void *);
> void *private;
> - void (*irq_func)(void *);
> + irqreturn_t (*irq_func)(void *);
> unsigned int flags;
> struct pardevice *next;
> struct pardevice *prev;
> @@ -298,7 +298,7 @@ extern void parport_put_port (struct parport *);
> struct pardevice *parport_register_device(struct parport *port,
> const char *name,
> int (*pf)(void *), void (*kf)(void *),
> - void (*irq_func)(void *),
> + irqreturn_t (*irq_func)(void *),
> int flags, void *handle);
>
> /* parport_unregister unlinks a device from the chain. */
> @@ -429,13 +429,17 @@ extern void parport_daisy_deselect_all (struct parport *port);
> extern int parport_daisy_select (struct parport *port, int daisy, int mode);
>
> /* Lowlevel drivers _can_ call this support function to handle irqs. */
> -static inline void parport_generic_irq(struct parport *port)
> +static inline irqreturn_t parport_generic_irq(struct parport *port)
> {
> + irqreturn_t ret = IRQ_NONE;
> +
> parport_ieee1284_interrupt (port);
> read_lock(&port->cad_lock);
> if (port->cad && port->cad->irq_func)
> - port->cad->irq_func(port->cad->private);
> + ret = port->cad->irq_func(port->cad->private);
> read_unlock(&port->cad_lock);
> +
> + return ret;
> }
>
> /* Prototypes from parport_procfs */
> --
> 2.1.0
>
--
With best regards,
Matwey V. Kornilov.
Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia
^ permalink raw reply
* [PATCH] Input: atkbd - correct MSC_SCAN events for force_release keys
From: Stefan Brüns @ 2014-09-28 21:13 UTC (permalink / raw)
To: linux-input
Without the change either no scancode would be reported on release
of force_release keys, or - if the key is marked as force_release
erroneously - the release event and the scancode would be reported
in separate reports to the input layer.
Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
---
drivers/input/keyboard/atkbd.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index 2dd1d0d..6375ae6 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -456,8 +456,9 @@ static irqreturn_t atkbd_interrupt(struct serio *serio, unsigned char data,
keycode = atkbd->keycode[code];
- if (keycode != ATKBD_KEY_NULL)
- input_event(dev, EV_MSC, MSC_SCAN, code);
+ if (!(atkbd->release && test_bit(code, atkbd->force_release_mask)))
+ if (keycode != ATKBD_KEY_NULL)
+ input_event(dev, EV_MSC, MSC_SCAN, code);
switch (keycode) {
case ATKBD_KEY_NULL:
@@ -511,6 +512,7 @@ static irqreturn_t atkbd_interrupt(struct serio *serio, unsigned char data,
input_sync(dev);
if (value && test_bit(code, atkbd->force_release_mask)) {
+ input_event(dev, EV_MSC, MSC_SCAN, code);
input_report_key(dev, keycode, 0);
input_sync(dev);
}
--
1.8.4.5
--
Stefan Brüns / Bergstraße 21 / 52062 Aachen
home: +49 241 53809034 mobile: +49 151 50412019
work: +49 2405 49936-424
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH v3 1/3] cap11xx: make driver generic for variant support
From: Matt Ranostay @ 2014-09-29 2:45 UTC (permalink / raw)
To: Daniel Mack
Cc: Kumar Gala, Dmitry Torokhov, Daniel Mack, linux-input,
linux-kernel@vger.kernel.org, Rob Herring,
devicetree@vger.kernel.org
In-Reply-To: <54270032.5010204@zonque.org>
On Sat, Sep 27, 2014 at 11:21 AM, Daniel Mack <daniel@zonque.org> wrote:
> On 09/25/2014 07:24 AM, Matt Ranostay wrote:
>> cap1106 driver can support much more one device make the driver
>> generic for support of similiar parts.
>>
>> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
>> ---
>> .../devicetree/bindings/input/cap1106.txt | 53 ----
>> .../devicetree/bindings/input/cap11xx.txt | 54 ++++
>> drivers/input/keyboard/Kconfig | 8 +-
>> drivers/input/keyboard/Makefile | 2 +-
>> drivers/input/keyboard/cap1106.c | 341 ---------------------
>> drivers/input/keyboard/cap11xx.c | 340 ++++++++++++++++++++
>> 6 files changed, 399 insertions(+), 399 deletions(-)
>
> As I've said, such a patch is really a whole lot more readable if you
> use the -M switch for git format-patch to detect renames.
>
TIL I learned of new git format-patch option. Will resubmit v4.
> Other than that, the series now looks good to me. I can't test it,
> however, as I don't have access to the hardware anymore.
>
Yeah I can't confirm it works on cap1106 but there is no reason it
wouldn't work.
Cap1188 works as expected.
Thanks,
Matt
> Reviewed-by: Daniel Mack <daniel@zonque.org>
>
>
>
> Thanks,
> Daniel
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox