From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 26D60C28CBC for ; Sat, 9 May 2020 12:55:36 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 07B9720820 for ; Sat, 9 May 2020 12:55:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727840AbgEIMzf (ORCPT ); Sat, 9 May 2020 08:55:35 -0400 Received: from fieber.vanmierlo.com ([84.243.197.177]:36055 "EHLO kerio9.vanmierlo.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726891AbgEIMzf (ORCPT ); Sat, 9 May 2020 08:55:35 -0400 X-Greylist: delayed 803 seconds by postgrey-1.27 at vger.kernel.org; Sat, 09 May 2020 08:55:34 EDT X-Footer: dmFubWllcmxvLmNvbQ== Received: from roundcube.vanmierlo.com ([192.168.37.37]) (authenticated user m.brock@vanmierlo.com) by kerio9.vanmierlo.com (Kerio Connect 9.2.12 patch 1) with ESMTPA; Sat, 9 May 2020 14:55:17 +0200 MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Sat, 09 May 2020 14:55:17 +0200 From: Maarten Brock To: Daniel Mack Cc: devicetree@vger.kernel.org, linux-serial@vger.kernel.org, gregkh@linuxfoundation.org, robh+dt@kernel.org, jslaby@suse.com, pascal.huerst@gmail.com, linux-serial-owner@vger.kernel.org Subject: Re: [PATCH 4/4] sc16is7xx: Use threaded IRQ In-Reply-To: <20200508143757.2609740-5-daniel@zonque.org> References: <20200508143757.2609740-1-daniel@zonque.org> <20200508143757.2609740-5-daniel@zonque.org> Message-ID: <61fdcf12976c924fd86c5203aba673a7@vanmierlo.com> X-Sender: m.brock@vanmierlo.com User-Agent: Roundcube Webmail/1.3.3 Sender: linux-serial-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-serial@vger.kernel.org On 2020-05-08 16:37, Daniel Mack wrote: > Use a threaded IRQ handler to get rid of the irq_work kthread. > This also allows for the driver to use interrupts generated by > a threaded controller. > > Signed-off-by: Daniel Mack > --- > drivers/tty/serial/sc16is7xx.c | 18 +++++------------- > 1 file changed, 5 insertions(+), 13 deletions(-) > > diff --git a/drivers/tty/serial/sc16is7xx.c > b/drivers/tty/serial/sc16is7xx.c > index 0997a5cac02a..e3c5b9501764 100644 > --- a/drivers/tty/serial/sc16is7xx.c > +++ b/drivers/tty/serial/sc16is7xx.c > @@ -328,7 +328,6 @@ struct sc16is7xx_port { > unsigned char buf[SC16IS7XX_FIFO_SIZE]; > struct kthread_worker kworker; > struct task_struct *kworker_task; > - struct kthread_work irq_work; > struct mutex efr_lock; > struct sc16is7xx_one p[0]; > }; > @@ -711,9 +710,9 @@ static bool sc16is7xx_port_irq(struct > sc16is7xx_port *s, int portno) > return true; > } > > -static void sc16is7xx_ist(struct kthread_work *ws) > +static irqreturn_t sc16is7xx_irq(int irq, void *dev_id) > { > - struct sc16is7xx_port *s = to_sc16is7xx_port(ws, irq_work); > + struct sc16is7xx_port *s = (struct sc16is7xx_port *)dev_id; > > mutex_lock(&s->efr_lock); > > @@ -728,13 +727,6 @@ static void sc16is7xx_ist(struct kthread_work *ws) > } > > mutex_unlock(&s->efr_lock); > -} > - > -static irqreturn_t sc16is7xx_irq(int irq, void *dev_id) > -{ > - struct sc16is7xx_port *s = (struct sc16is7xx_port *)dev_id; > - > - kthread_queue_work(&s->kworker, &s->irq_work); > > return IRQ_HANDLED; > } > @@ -1230,7 +1222,6 @@ static int sc16is7xx_probe(struct device *dev, > mutex_init(&s->efr_lock); > > kthread_init_worker(&s->kworker); > - kthread_init_work(&s->irq_work, sc16is7xx_ist); > s->kworker_task = kthread_run(kthread_worker_fn, &s->kworker, > "sc16is7xx"); > if (IS_ERR(s->kworker_task)) { > @@ -1317,8 +1308,9 @@ static int sc16is7xx_probe(struct device *dev, > } > > /* Setup interrupt */ > - ret = devm_request_irq(dev, irq, sc16is7xx_irq, > - IRQF_TRIGGER_FALLING, dev_name(dev), s); > + ret = devm_request_threaded_irq(dev, irq, NULL, sc16is7xx_irq, > + IRQF_TRIGGER_FALLING | IRQF_ONESHOT, > + dev_name(dev), s); > if (!ret) > return 0; Since UART0 is first handled completely in the for loop before UART1 is handled, a new interrupt may arise on UART0 while UART1 is being handled. The result is a missed interrupt since the IRQ line might not *FALL* again. Therefor I suggest to change IRQF_TRIGGER_FALLING to IRQF_TRIGGER_LOW. This way the thread will be retriggered after IRQ_HANDLED is returned. Maarten