From patchwork Mon Jul 25 21:01:08 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dmitry Torokhov X-Patchwork-Id: 9246521 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 630F66077C for ; Mon, 25 Jul 2016 21:01:34 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 535A5276AE for ; Mon, 25 Jul 2016 21:01:34 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 473D527813; Mon, 25 Jul 2016 21:01:34 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 47D15276AE for ; Mon, 25 Jul 2016 21:01:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752903AbcGYVBa (ORCPT ); Mon, 25 Jul 2016 17:01:30 -0400 Received: from mail-pa0-f65.google.com ([209.85.220.65]:35062 "EHLO mail-pa0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752821AbcGYVBM (ORCPT ); Mon, 25 Jul 2016 17:01:12 -0400 Received: by mail-pa0-f65.google.com with SMTP id cf3so11985365pad.2 for ; Mon, 25 Jul 2016 14:01:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=4JmoidQAffFqZw06ccd74B0jt8g96kwk9NzFhmL7U6g=; b=gtVb++KEUwigjNDneNOPPLeUEuW5zjRpDweHSRpwiAO6gXRicPURI5v/D5Cdm4BBLb AWvg0+jYYjfve4oqYI0/0BNyaadAdPMBJqPoT6lXuHUlJXjVi0WBO0/P3LE3kG6DyR/e vHXD1c3xYtyREn8QHbVZQr869t99mIFnNJsP822EUFr7m934djuZD0W0AC9oHhcRCWt3 w0uyJhtPfn9yDYOKKFjR57Mbq9cB8f4qpapoWmVfwdXKwOBjspw9n+GZTndXIAPUJWpr 12Id+xFbk+zxm1Uq5sKpdIXf0T+2iI+E3SNP+p2Dhr9FT7gnZnWk2jHdxt5D8UNqqVVy PDFw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=4JmoidQAffFqZw06ccd74B0jt8g96kwk9NzFhmL7U6g=; b=IYaIoqSRkaLPvg/m5T74fZxU5MkKfrpEkqeeBSgQmg9fKGUv7OJRI/mIQQ9+ogVn99 r9Hu3X8W5w0SXJq0w3nkIheHNx2GGywCuvBvKpoNY3mdxq1I9tLpUKepLvMhZiCaQnZW UFVYYMk2dctjVGKJZ3lqKSGgqbfSBj/ReF0k2Wz3aZZwVDwkTOw35UpRNYk8yer3nFsh 6HgsaBkqdw/cL4oPfEOIhxfG76vdH3rZgR2D+gmlp9ne22OeJ7nn6y4Wkx8SFJPZhP5B 72YGmo09Jpd0ftsus7VPzRRMgdHzomkQwFJWJwTO1hgNlbcZ+x49bmL5GNDGuAr59PuY a+KQ== X-Gm-Message-State: AEkoouta54PHBPAAs9T+eE18krXJPcabVe3CQMCj8r6r2UH0zaBTY4R3nWfRsly4Y4Uodg== X-Received: by 10.66.182.232 with SMTP id eh8mr32971247pac.146.1469480470829; Mon, 25 Jul 2016 14:01:10 -0700 (PDT) Received: from dtor-ws ([2620:0:1000:1311:5d1:e056:3ff1:7da3]) by smtp.gmail.com with ESMTPSA id p14sm42375610par.32.2016.07.25.14.01.09 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Mon, 25 Jul 2016 14:01:10 -0700 (PDT) Date: Mon, 25 Jul 2016 14:01:08 -0700 From: Dmitry Torokhov To: Mark Laws Cc: KY Srinivasan , "Van De Ven, Arjan" , Linux Input Subject: Re: [PATCH] Input: i8042 - Fix console keyboard support on Gen2 Hyper-V VMs Message-ID: <20160725210108.GK27415@dtor-ws> References: 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 X-Virus-Scanned: ClamAV using ClamSMTP [ resending to inlude the list ] Hi Mark, On Tue, Jul 19, 2016 at 06:22:57PM -0700, Mark Laws wrote: > On Tue, Jul 19, 2016 at 4:34 PM, KY Srinivasan wrote: > >> -----Original Message----- > >> From: Mark Laws [mailto:mdl@60hz.org] > >> Sent: Tuesday, July 19, 2016 1:29 AM > >> To: Van De Ven, Arjan > >> Cc: KY Srinivasan > >> Subject: Re: [PATCH] Input: i8042 - Fix console keyboard support on Gen2 > >> Hyper-V VMs > >> > >> On Tue, Jun 21, 2016 at 7:41 PM, Mark Laws wrote: > >> > On Wed, Jun 22, 2016 at 11:36 AM, Van De Ven, Arjan > >> > wrote: > >> >>> Hi KY and Arjan, > >> >>> > >> >>> Does anything remain to be fixed in this patch? > >> >> > >> >> it works great for me.... 8042 is no longer on my radar of trouble makers... > >> > > >> > Glad to hear it. I hope it can get merged soon, as it's surely been a > >> > nuisance for at least a few other folks. > >> > >> Hi, > >> > >> Is there anyone I should ping about getting this merged? Should I CC > >> Dmitry again? > > > > Please do. > > > > K. Y > > Hi Dmitry, > > Could you please take a look at the patch earlier in this thread and > merge it if it's OK? K.Y. and Arjan have said it's good. I can provide > a rebased version if needed, though I think the one from this thread > should apply cleanly. Sorry for the delay, the reason is that I absolutely hated exporting the ports array from i8042 and into yet another module. Even though I was the author of i8042_check_port_owner() I do not like this mechanism at all and I think it is worse than doing a small layer violation and having a shared PS/2 mutex directly in serio port structure. Can you please tell me if the patch below solves the issue for you? Thanks! diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c index 4541957..b4d3408 100644 --- a/drivers/input/serio/i8042.c +++ b/drivers/input/serio/i8042.c @@ -1277,6 +1277,7 @@ static int __init i8042_create_kbd_port(void) serio->start = i8042_start; serio->stop = i8042_stop; serio->close = i8042_port_close; + serio->ps2_cmd_mutex = &i8042_mutex; serio->port_data = port; serio->dev.parent = &i8042_platform_device->dev; strlcpy(serio->name, "i8042 KBD port", sizeof(serio->name)); @@ -1373,21 +1374,6 @@ static void i8042_unregister_ports(void) } } -/* - * Checks whether port belongs to i8042 controller. - */ -bool i8042_check_port_owner(const struct serio *port) -{ - int i; - - for (i = 0; i < I8042_NUM_PORTS; i++) - if (i8042_ports[i].serio == port) - return true; - - return false; -} -EXPORT_SYMBOL(i8042_check_port_owner); - static void i8042_free_irqs(void) { if (i8042_aux_irq_registered) diff --git a/drivers/input/serio/libps2.c b/drivers/input/serio/libps2.c index 316f2c8..83e9c66 100644 --- a/drivers/input/serio/libps2.c +++ b/drivers/input/serio/libps2.c @@ -56,19 +56,17 @@ EXPORT_SYMBOL(ps2_sendbyte); void ps2_begin_command(struct ps2dev *ps2dev) { - mutex_lock(&ps2dev->cmd_mutex); + struct mutex *m = ps2dev->serio->ps2_cmd_mutex ?: &ps2dev->cmd_mutex; - if (i8042_check_port_owner(ps2dev->serio)) - i8042_lock_chip(); + mutex_lock(m); } EXPORT_SYMBOL(ps2_begin_command); void ps2_end_command(struct ps2dev *ps2dev) { - if (i8042_check_port_owner(ps2dev->serio)) - i8042_unlock_chip(); + struct mutex *m = ps2dev->serio->ps2_cmd_mutex ?: &ps2dev->cmd_mutex; - mutex_unlock(&ps2dev->cmd_mutex); + mutex_unlock(m); } EXPORT_SYMBOL(ps2_end_command); diff --git a/include/linux/i8042.h b/include/linux/i8042.h index 0f9bafa..d98780c 100644 --- a/include/linux/i8042.h +++ b/include/linux/i8042.h @@ -62,7 +62,6 @@ struct serio; void i8042_lock_chip(void); void i8042_unlock_chip(void); int i8042_command(unsigned char *param, int command); -bool i8042_check_port_owner(const struct serio *); int i8042_install_filter(bool (*filter)(unsigned char data, unsigned char str, struct serio *serio)); int i8042_remove_filter(bool (*filter)(unsigned char data, unsigned char str, @@ -83,11 +82,6 @@ static inline int i8042_command(unsigned char *param, int command) return -ENODEV; } -static inline bool i8042_check_port_owner(const struct serio *serio) -{ - return false; -} - static inline int i8042_install_filter(bool (*filter)(unsigned char data, unsigned char str, struct serio *serio)) { diff --git a/include/linux/serio.h b/include/linux/serio.h index df4ab5d..c733cff 100644 --- a/include/linux/serio.h +++ b/include/linux/serio.h @@ -31,7 +31,8 @@ struct serio { struct serio_device_id id; - spinlock_t lock; /* protects critical sections from port's interrupt handler */ + /* Protects critical sections from port's interrupt handler */ + spinlock_t lock; int (*write)(struct serio *, unsigned char); int (*open)(struct serio *); @@ -40,16 +41,29 @@ struct serio { void (*stop)(struct serio *); struct serio *parent; - struct list_head child_node; /* Entry in parent->children list */ + /* Entry in parent->children list */ + struct list_head child_node; struct list_head children; - unsigned int depth; /* level of nesting in serio hierarchy */ + /* Level of nesting in serio hierarchy */ + unsigned int depth; - struct serio_driver *drv; /* accessed from interrupt, must be protected by serio->lock and serio->sem */ - struct mutex drv_mutex; /* protects serio->drv so attributes can pin driver */ + /* + * serio->drv is accessed from interrupt handlers; when modifying + * caller should acquire serio->drv_mutex and serio->lock. + */ + struct serio_driver *drv; + /* Protects serio->drv so attributes can pin current driver */ + struct mutex drv_mutex; struct device dev; struct list_head node; + + /* + * For use by PS/2 layer when several ports share hardware and + * may get indigestion when exposed to concurrent access (i8042). + */ + struct mutex *ps2_cmd_mutex; }; #define to_serio_port(d) container_of(d, struct serio, dev)