Received: by 2002:a05:7412:8d10:b0:f3:1519:9f41 with SMTP id bj16csp6650241rdb; Fri, 15 Dec 2023 04:58:16 -0800 (PST) X-Google-Smtp-Source: AGHT+IF26czCH4grfN2cEkGYYmNLkWLmvUzAouK6CL4cHfBt+oU1O9p9Q3kr5yTQGwyUBqdbvhQF X-Received: by 2002:a05:6214:224d:b0:67a:a721:b188 with SMTP id c13-20020a056214224d00b0067aa721b188mr16460779qvc.67.1702645096624; Fri, 15 Dec 2023 04:58:16 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1702645096; cv=none; d=google.com; s=arc-20160816; b=nB6QAVZmxDxLNYmsmLsTlULd6E9UtO1vaiiEC61QwxbMRtIeZnDagyPjZjiRBGV0JZ zBRtiTOQpgxy8SzXyHx5Cu6BQwksYWnwxNldcsCUMJkd/XYN+T5bvBvCykIVaVtYKs65 JlXs2UyhTej8iAumJcObTfXVus2UUbxT4WT58eMxpK6zJbI/zUzl9bYbKZbudKESQJpa LamtA5bQwGD6xVAJUcRWorrHiO2dfvnWdklvJIK1i3kyNduMgWgtdmiDZh0+bnFxeCa8 xGLptA4QXOqBivqHh3xF8l0yQ/OwZaqUkSH6Sy4UhmMBapKkRaul/Ou7ZSuJM88AipIH XRvw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=in-reply-to:content-disposition:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:references:message-id:subject:cc :to:from:date:dkim-signature; bh=SxQUqw1Lm+f1JkkxgaJEJfwFFhEdNqCTWcPVADcjmLY=; fh=Kcjojbf2XXtiC+Wn1xevRdkklCQiQQiXz1orxX2A6Yo=; b=ag2KZ6WMLGbKpc/AfuZsiYihtW+oPbzRRBq8bTKvDzKzZ95HTAx/QnZ1NDnd1bFChp ZIUKOv6wKUbjyq6TS9hNKQthz5P1WXRB94Dt2fHNi1qJBeR06NZ9qYTA/cxuhZ6ELqM7 cE8+gNv4OJJaZsqnortusXEnWSbN4EbbmWfbqUmSaTu9g1QXzvQ9PL8UW9b7JQ9zGJqI 4MZGN9m9BoFsM0bev3MYgUrwLEzICGcy2u0jRGNqm3GDf6zJlUvi11yhAR57LqQMgOmQ 4uHHxe9Bd6K7VygrcanqrinHd4gQN34Sn5SmloXSy/2kEOEgsd7doJL6CU49m1oLnRtk oK/w== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=GeqDRdKf; spf=pass (google.com: domain of linux-kernel+bounces-965-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) smtp.mailfrom="linux-kernel+bounces-965-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from ny.mirrors.kernel.org (ny.mirrors.kernel.org. [147.75.199.223]) by mx.google.com with ESMTPS id h20-20020a05620a245400b0077f60a93b5esi12895478qkn.649.2023.12.15.04.58.16 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 15 Dec 2023 04:58:16 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-965-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=GeqDRdKf; spf=pass (google.com: domain of linux-kernel+bounces-965-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) smtp.mailfrom="linux-kernel+bounces-965-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ny.mirrors.kernel.org (Postfix) with ESMTPS id 54A7F1C23755 for ; Fri, 15 Dec 2023 12:58:16 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id E2D862D056; Fri, 15 Dec 2023 12:57:58 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="GeqDRdKf" X-Original-To: linux-kernel@vger.kernel.org Received: from mail-lf1-f47.google.com (mail-lf1-f47.google.com [209.85.167.47]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 715FD2EB19; Fri, 15 Dec 2023 12:57:56 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-lf1-f47.google.com with SMTP id 2adb3069b0e04-50bf2d9b3fdso745422e87.3; Fri, 15 Dec 2023 04:57:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1702645074; x=1703249874; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=SxQUqw1Lm+f1JkkxgaJEJfwFFhEdNqCTWcPVADcjmLY=; b=GeqDRdKfctVkl2pPCH28Y7H+yPFUNckY2DpBWboPL6YOFSnQT4xxsZOr0U06oNhXwe sW2N2LpGgCEHMBYwB8vGefYbMql6UjQjzaZg1Tx2m97PPcYNNXBkGkXCTJHNSS/YnS71 n8rMpk8tsEfX9YFIIgp5TIvhhxLRlWllgyCWqm8SYMAJORal8/8vDn5tpD8KtkJ5qd8r oC2GrIvSFGGRX8ypLlFPNEJzTrKIvB9nhIkbkRdJUYv4IWxKVM+aYWjdzbX4p1UcDr3K hatJKIwSNKUsYsGrDiWwuFPZtVbzWFg3KrlmV2jyekady2BnV5gHfDA43pD4SXxWHFEx CmnQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702645074; x=1703249874; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=SxQUqw1Lm+f1JkkxgaJEJfwFFhEdNqCTWcPVADcjmLY=; b=MWtr9Tg5QRTdm3Hixq0UATqHRGlD0J5RCQPMUKB3y5Ip9YhV5o3oWye7f0bbObCMIS e1f3zIPZDZjvu1zZPrLbJh2YD+8SLcuxGNY6N8CPoJVTjztBvLULE5SvdDNqORzamPIQ fPJHDqRZcgDDnIYqXG+xYAsXixi3R44TlYuoAHYOHGruvf8asUbiBcsDdEPojL0jUW8z QWi4RmMYetpOIgEzkXUXFrsLI0MA62qPrxMfMjZL9Wx+m0HvKErMLA9rKgiLxAv4b2/j rKQ2Gs1DZNceHqDIXChr7awDJQYcxScdvMqXBysZ2z053ZLzeLN9uHEkuyUE1OVCk2SV BCpw== X-Gm-Message-State: AOJu0YwOXYUxHF3+41VP/FZgn/8x2XvspMBtxZ32z+P3HEWwiyWwIad1 qpBA91NDg01lk62U5wmhQgA= X-Received: by 2002:ac2:5497:0:b0:50b:f006:3f92 with SMTP id t23-20020ac25497000000b0050bf0063f92mr5227019lfk.134.1702645074149; Fri, 15 Dec 2023 04:57:54 -0800 (PST) Received: from mobilestation ([178.176.56.174]) by smtp.gmail.com with ESMTPSA id w22-20020ac254b6000000b0050bf273a895sm2129201lfk.240.2023.12.15.04.57.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 15 Dec 2023 04:57:53 -0800 (PST) Date: Fri, 15 Dec 2023 15:57:51 +0300 From: Serge Semin To: Thomas Gleixner , xiongxin Cc: Linus Walleij , Luo Jiaxing , Marc Zyngier , Bartosz Golaszewski , Serge Semin , Andy Shevchenko , Andy Shevchenko , "open list:GPIO SUBSYSTEM" , "linux-kernel@vger.kernel.org" , Linuxarm Subject: Re: [PATCH v1] gpio: dwapb: mask/unmask IRQ when disable/enable it Message-ID: References: <1606728979-44259-1-git-send-email-luojiaxing@huawei.com> <87fs03opju.ffs@tglx> <87a5qbohtp.ffs@tglx> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87a5qbohtp.ffs@tglx> On Fri, Dec 15, 2023 at 11:56:02AM +0100, Thomas Gleixner wrote: > On Fri, Dec 15 2023 at 13:24, Serge Semin wrote: > > On Fri, Dec 15, 2023 at 09:09:09AM +0100, Thomas Gleixner wrote: > >> On Sat, Dec 05 2020 at 22:58, Linus Walleij wrote: > >> > Sorry for top posting but I need the help of the irqchip maintainer > >> > Marc Z to hash this out. > >> > > >> > The mask/unmask/disable/enable semantics is something that > >> > you need to work with every day to understand right. > >> > >> The patch is correct. > >> > >> The irq_enable() callback is required to be a superset of > >> irq_unmask(). I.e. the core code expects it to do: > >> > >> 1) Some preparatory work to enable the interrupt line > >> > >> 2) Unmask the interrupt, which is why the masked state is cleared > >> by the core after invoking the irq_enable() callback. > >> > >> #2 is pretty obvious because if an interrupt chip does not implement the > >> irq_enable() callback the core defaults to irq_unmask() > >> > >> Correspondingly the core expects from the irq_disable() callback: > >> > >> 1) To mask the interrupt > >> > >> 2) To do some extra work to disable the interrupt line > >> > >> Same reasoning as above vs. #1 as the core fallback is to invoke the > >> irq_unmask() callback when the irq_disable() callback is not > >> implemented. > > > > Just curious. Wouldn't that be more correct/portable for the core to > > call both callbacks when it's required and if both are provided? So > > the supersetness requirement would be no longer applied to the > > IRQ enable/disable callbacks implementation thus avoiding the code > > duplications in the low-level drivers. > > We could do that, but there are chips which require atomicity of the > operations (#1/#2). Not sure whether it safes much. I see. Thanks for the answer. Right, seeing there are only three GPIO drivers have such problem: drivers/gpio/gpio-ml-ioh.c drivers/gpio/gpio-dwapb.c drivers/gpio/gpio-hisi.c it's better to leave the semantics as is. It just isn't worth it to risk breaking so many platforms due to several drivers. Regarding this patch implementation. It can be optimized a bit: diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c index 4a4f61bf6c58..15943f67758c 100644 --- a/drivers/gpio/gpio-dwapb.c +++ b/drivers/gpio/gpio-dwapb.c @@ -282,13 +282,15 @@ static void dwapb_irq_enable(struct irq_data *d) { struct gpio_chip *gc = irq_data_get_irq_chip_data(d); struct dwapb_gpio *gpio = to_dwapb_gpio(gc); + u32 mask = BIT(irqd_to_hwirq(d)); unsigned long flags; u32 val; raw_spin_lock_irqsave(&gc->bgpio_lock, flags); - val = dwapb_read(gpio, GPIO_INTEN); - val |= BIT(irqd_to_hwirq(d)); + val = dwapb_read(gpio, GPIO_INTEN) | mask; dwapb_write(gpio, GPIO_INTEN, val); + val = dwapb_read(gpio, GPIO_INTMASK) & ~mask; + dwapb_write(gpio, GPIO_INTMASK, val); raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags); } @@ -296,12 +298,14 @@ static void dwapb_irq_disable(struct irq_data *d) { struct gpio_chip *gc = irq_data_get_irq_chip_data(d); struct dwapb_gpio *gpio = to_dwapb_gpio(gc); + u32 mask = BIT(irqd_to_hwirq(d)); unsigned long flags; u32 val; raw_spin_lock_irqsave(&gc->bgpio_lock, flags); - val = dwapb_read(gpio, GPIO_INTEN); - val &= ~BIT(irqd_to_hwirq(d)); + val = dwapb_read(gpio, GPIO_INTMASK) | mask; + dwapb_write(gpio, GPIO_INTMASK, val); + val = dwapb_read(gpio, GPIO_INTEN) & ~mask; dwapb_write(gpio, GPIO_INTEN, val); raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags); } Seeing Luo' email bounces back, Xiang (sorry if I spell your name incorrectly), could you please test the patch above out whether it fixes your problem, and then resubmit it? Please don't forget to add a Fixes tag. I guess the problem has been here since the driver birthday: Fixes: 7779b3455697 ("gpio: add a driver for the Synopsys DesignWare APB GPIO block") -Serge(y) > > Thanks, > > tglx