From patchwork Mon Jul 16 20:59:37 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Henrik Rydberg X-Patchwork-Id: 1202031 X-Patchwork-Delegate: jikos@jikos.cz Return-Path: X-Original-To: patchwork-linux-input@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork2.kernel.org (Postfix) with ESMTP id 56783E0038 for ; Mon, 16 Jul 2012 20:59:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752291Ab2GPU7J (ORCPT ); Mon, 16 Jul 2012 16:59:09 -0400 Received: from smtprelay-b11.telenor.se ([62.127.194.20]:50695 "EHLO smtprelay-b11.telenor.se" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751919Ab2GPU7I (ORCPT ); Mon, 16 Jul 2012 16:59:08 -0400 Received: from ipb4.telenor.se (ipb4.telenor.se [195.54.127.167]) by smtprelay-b11.telenor.se (Postfix) with ESMTP id 451AEDD11 for ; Mon, 16 Jul 2012 22:59:06 +0200 (CEST) X-SENDER-IP: [85.230.170.20] X-LISTENER: [smtp.bredband.net] X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: AmtbAHGABFBV5qoUPGdsb2JhbABFiiSrQoNqGQEBAQE3NIIgAQEEAScTHCMFCwgDDh8ZFCUKGogaCrwNFI5LgkhgA5U6hWaDQYlB X-IronPort-AV: E=Sophos;i="4.77,596,1336341600"; d="scan'208";a="81463935" Received: from c-14aae655.710-13-64736c12.cust.bredbandsbolaget.se (HELO polaris) ([85.230.170.20]) by ipb4.telenor.se with SMTP; 16 Jul 2012 22:58:50 +0200 Received: by polaris (sSMTP sendmail emulation); Mon, 16 Jul 2012 22:59:37 +0200 From: "Henrik Rydberg" Date: Mon, 16 Jul 2012 22:59:37 +0200 To: David Herrmann Cc: Jiri Kosina , linux-input@vger.kernel.org Subject: Re: [PATCH 1/3] HID: Add HID_CLAIMED_OTHER for non-generic drivers Message-ID: <20120716205937.GA603@polaris.bitmath.org> References: <20120716184530.GA407@polaris.bitmath.org> <20120716193502.GA617@polaris.bitmath.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-input-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org > I am sorry, I misunderstood you. Yes, in fact, this is what wiimote > currently does. Well, it uses HID_CONNECT_HIDRAW but this has no > effect if CONFIG_HIDRAW is not set so this is equivalent to 0. My > first attempt was to make this work, however, this means refactoring > hid_connect() a lot as we need to differentiate between > hidraw_connect() failing and HID_CONNECT_HIDRAW being not set. Same > for hidinput_connect() and hiddev_connect(). That is, if > hid_hw_start() is called with HID_CONNECT_HIDDEV set, but > hiddev_connect() fails? Should the core bail out or let the device > through? In most cases bailing out is the best option. However, what > to do for the wiimote case? It requests hidraw but if hidraw_connect() > fails, then the wiimote driver can still work without it so in this > case we must not bail out. > > Taking this into account I really have no idea how to implement this > in a cleaner and safer way than using HID_CONNECT_OTHER. Btw., what > should the wiimote driver pass to hid_hw_start() in your case? It > wants HID_CONNECT_HIDRAW but also wants to get through if > CONFIG_HIDRAW is not set. It cannot pass 0 as this would always > disable HIDRAW. But in the case that HIDRAW is not available, it > cannot tell the core that it wants to stay on the bus. Hence, I think > using HID_CONNECT_OTHER is the only way, isn't it? > > > To catch possible mistakes, one could check for the presence of > > raw_event() instead, for instance. > > > > What I am getting at is that we really do not need to create any more > > backdoors into the hid core - on the contrary, we can most likely > > easily remove some of them instead. > > I fully agree, but I have to admit that I didn't find an easier way > that actually works. > > Thanks a lot for having a look at this. If you have any other ideas I > will gladly implement and test them, but I am currently out of ideas. Would something like this work for you? Henrik --- 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 diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 4c87276..a43e14c 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -1373,8 +1373,8 @@ int hid_connect(struct hid_device *hdev, unsigned int connect_mask) if ((connect_mask & HID_CONNECT_HIDRAW) && !hidraw_connect(hdev)) hdev->claimed |= HID_CLAIMED_HIDRAW; - if (!hdev->claimed) { - hid_err(hdev, "claimed by neither input, hiddev nor hidraw\n"); + if (!hdev->claimed && !hdev->driver->raw_event) { + hid_err(hdev, "device has no listeners, quitting\n"); return -ENODEV; } diff --git a/drivers/hid/hid-picolcd.c b/drivers/hid/hid-picolcd.c index 45c3433..74c388d 100644 --- a/drivers/hid/hid-picolcd.c +++ b/drivers/hid/hid-picolcd.c @@ -2613,11 +2613,7 @@ static int picolcd_probe(struct hid_device *hdev, goto err_cleanup_data; } - /* We don't use hidinput but hid_hw_start() fails if nothing is - * claimed. So spoof claimed input. */ - hdev->claimed = HID_CLAIMED_INPUT; error = hid_hw_start(hdev, 0); - hdev->claimed = 0; if (error) { hid_err(hdev, "hardware start failed\n"); goto err_cleanup_data;