From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752727AbeBCQbq (ORCPT ); Sat, 3 Feb 2018 11:31:46 -0500 Received: from m15-42.126.com ([220.181.15.42]:10893 "EHLO m15-42.126.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752026AbeBCQbg (ORCPT ); Sat, 3 Feb 2018 11:31:36 -0500 X-Originating-IP: [123.139.75.126] Date: Sun, 4 Feb 2018 00:31:29 +0800 (CST) From: "Zhang Bo" To: "Marcus Folkesson" Cc: dmitry.torokhov@gmail.com, DRivshin@allworx.com, robh@kernel.org, linux-input@vger.kernel.org, linux-kernel@vger.kernel.org, zhang.bo19@zte.com.cn Subject: Re: [PATCH] Input: matrix_keypad - fix keypad does not response X-Priority: 3 X-Mailer: Coremail Webmail Server Version SP_ntes V3.5 build 20160729(86883.8884) Copyright (c) 2002-2018 www.mailtech.cn 126com In-Reply-To: <20180203134450.GC707@gmail.com> References: <20180203120046.10988-1-zbsdta@126.com> <20180203134450.GC707@gmail.com> Content-Type: text/plain; charset=GBK MIME-Version: 1.0 Message-ID: <3527eac2.37b3.1615c822887.Coremail.zbsdta@126.com> X-Coremail-Locale: zh_CN X-CM-TRANSID: KsqowABnGL_h43Va3lmRAA--.33772W X-CM-SenderInfo: h2evv3bd6rjloofrz/1tbijgHhSVpD3+f9kwAAso X-Coremail-Antispam: 1U5529EdanIXcx71UUUUU7vcSsGvfC2KfnxnUU== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id w13GVrig025195 At 2018-02-03 21:44:50, "Marcus Folkesson" wrote: >> diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c >> index 1f316d66e6f7..13fe51824637 100644 >> --- a/drivers/input/keyboard/matrix_keypad.c >> +++ b/drivers/input/keyboard/matrix_keypad.c >> @@ -169,7 +169,8 @@ static void matrix_keypad_scan(struct work_struct *work) >> /* Enable IRQs again */ >> spin_lock_irq(&keypad->lock); >> keypad->scan_pending = false; >> - enable_row_irqs(keypad); >> + if (keypad->stopped == false) >> + enable_row_irqs(keypad); >> spin_unlock_irq(&keypad->lock); >> } >> >> @@ -202,14 +203,16 @@ static int matrix_keypad_start(struct input_dev *dev) >> { >> struct matrix_keypad *keypad = input_get_drvdata(dev); >> >> + spin_lock_irq(&keypad->lock); >> keypad->stopped = false; >> - mb(); >> >> /* >> * Schedule an immediate key scan to capture current key state; >> * columns will be activated and IRQs be enabled after the scan. >> */ >> - schedule_delayed_work(&keypad->work, 0); >> + if (keypad->scan_pending == false) >> + schedule_delayed_work(&keypad->work, 0); >> + spin_unlock_irq(&keypad->lock); >> >> return 0; >> } >> @@ -218,14 +221,17 @@ static void matrix_keypad_stop(struct input_dev *dev) >> { >> struct matrix_keypad *keypad = input_get_drvdata(dev); >> >> + spin_lock_irq(&keypad->lock); >> keypad->stopped = true; >> - mb(); >> - flush_work(&keypad->work.work); >> /* >> * matrix_keypad_scan() will leave IRQs enabled; >> * we should disable them now. >> */ >> - disable_row_irqs(keypad); >> + if (keypad->scan_pending == false) >> + disable_row_irqs(keypad); >> + spin_unlock_irq(&keypad->lock); >> + >> + flush_work(&keypad->work.work); >> } > > >Hum, I think we should use spin_lock_irqsave/spin_lock_irqrestore >instead to be on the safe side. >I don't see how we could guarantee that irqs is allways enabled when >calling spin_lock_irq(). spin_lock_irq and spin_unlock_irq are called in matrix_keypad_stop and matrix_keypad_start which are in suspend and resume, they run in process context, matrix_keypad_scan runs in worker which is in process context also.