Message-ID: <CANq1E4TkgKsZcJAygY4k8rwvYMPku_uzWqhcgp+HkfMsb+n84Q@mail.gmail.com>
Date:	Fri, 9 Aug 2013 19:02:59 +0200
From:	David Herrmann <dh.herrmann@...il.com>
To:	Alexander Holler <holler@...oftware.de>
Cc:	rtc-linux@...glegroups.com, Jiri Kosina <jkosina@...e.cz>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	Alessandro Zummo <a.zummo@...ertech.it>,
	Andrew de los Reyes <adlr@...omium.org>
Subject: Re: [rtc-linux] Re: [PATCH 2/2 RESEND] rtc: rtc-hid-sensor-time:
 enable HID input processing early

Hi Alexander

On Fri, Aug 9, 2013 at 6:33 PM, Alexander Holler <holler@...oftware.de> wrote:
> Am 09.08.2013 18:21, schrieb Alexander Holler:
>
>> I've now also verified if hid-sensor-hub receives an event with
>> sensor_hub_raw_event() in the error-path (hid_device_io_stop() called
>> and probe() failed), and this still *does* happen. That event (input
>> report) doesn't come through hid-sensor-hub to my driver, but I think
>> this is because of my call to sensor_hub_remove_callback() which is in
>> the error path too.
>> So I actually wonder why the input report still is reported from the
>> hid-subsystem to hid-sensor-hub, even after I've called
>> hid_device_io_stop() and probe() failed.
>> Maybe everything is still ok and I just got confused with the somehow
>> complicate interactions between the usb- and hid-subsystem,
>> hid-sensor-hub (which uses MFD) and rtc-hid-sensor-time.
>
>
> Adding some more stuff to the confusion: Currently I think it is correct
> that hid-sensor-hub still receives the event, even after rtc-hid-sensor-time
> called hid_device_io_stop() and probe() failed. The reason the same reason,
> why hid-sensor-hub uses mfd, the actual hardware device might be shared by
> different drivers (therfor -hub).

I don't have time right know to debug this, but I thought I'd just
clarify how the HID I/O lock works:

HID core uses a semaphore to protect driver probe and removal. That
is, the semaphore is locked during the ->probe() and ->remove()
callbacks. The input event handler (in atomic context!) tries to lock
this, too. If it fails due to ->probe or ->remove currently running,
it simply drops the input events (which is fine for reasons that don't
matter here). If it can lock it, it simply calls the ->raw_event() or
whatever callbacks of the driver (probably via hid-input).
If no driver is currently bound to a device, all input events are
always dropped.

If a driver now needs to perform I/O during ->probe or ->remove, they
must explicitly notify HID core about this. We cannot allow it
automatically as drivers must have a chance to setup some context
before the first events are passed in.
We start I/O via hid_device_io_start(). This simply releases the
->probe() semaphore and makes sure HID core knows about this. Once you
call it, your drivers input callbacks will be used by HID core so you
can perform I/O. Once you call hid_device_io_stop() the semaphore is
locked again and no more I/O is possible.

Same applies to the ->remove() callback, although it's not used by any
driver, yet. The reason is that ->remove() is almost always called if
the transport layer is already closed so any I/O will return -EIO.

We make sure we don't do any double down() or up() by tracking it via
a boolean. The memory barriers there aren't really obvious but it
should be correct.

I hope that explains how all this works. I can look over your patch on
Sunday if still necessary.

Cheers
David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists