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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 84490ECAAD3 for ; Thu, 1 Sep 2022 18:12:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=iWi7gpKDQLVHO4m4/nlV4xDfKcMux/oYCXpKbwRhyeY=; b=juayAXbzatVcuA 4DdJmyo8/n5kFv1sY/uJr6ALD6E7XgJGnDcgZ6ESnCpak5qNclxGUZpl7m6hKfM33zdCq1FCVU4xy U36vvj1aOaMhSjfycubabaqYIvVszbh8zJH4h9nflO2ABdyruxHjVhjefk/mxFddsgmjRSHB/SGHW gsdTp8q1HeS+fpWOFW2Kd2lFszMmTRhAH6hF8NsdlwbysoDkTWfE6pEAHocd+c/Q/WhSNdBR/1UnA Yl1GWVUmWjbFy1BKQaZ/TqTgJwVFSs/1zEtoWpxwuqOzqsNYMy8VlrcRbI+KjlGA/NxCQAg1nBpYF 33GuCkjEck5lCdL51vsg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oToeP-00DpTK-Aa; Thu, 01 Sep 2022 18:10:50 +0000 Received: from mail-qt1-x82e.google.com ([2607:f8b0:4864:20::82e]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oToYH-00Dme2-Qn for linux-arm-kernel@lists.infradead.org; Thu, 01 Sep 2022 18:04:31 +0000 Received: by mail-qt1-x82e.google.com with SMTP id e28so14079044qts.1 for ; Thu, 01 Sep 2022 11:04:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date; bh=Zv8zSKFq6bzQqKXYqWy0Qx4amMNXqj8LcRMKjpftT8k=; b=KUOKej5nPOeGXo5nEfUAHYOLs5eyvNYlpPTgCZVwvX1z47fwyYmBooIcwdClVmNz/v BRlsK21Dt9FO/arMv4k1bBH9kpHYhGlhCYiHWwSs7zcTWAS1o68a1rng8ufeVyq64mCt p546eMwZIIcOP8bw44GqQ79ZHCkKG3OtwLGvXJ8UJulq9EvJ/SlNLAoghw4QreA9qeju 3+z4Yxdu9AcK3lIk8lLUAEA0Ts0X4QHz8YrmEABFlwU8pmOnUzx5OVD4tlF8JlFdgDWG G+M+vJxEIoIdPawxcqf3xok6F/M0ttOpxReuVfdwkBWLnB/drLA8iRY22Wj87RvL3w+E jGsQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date; bh=Zv8zSKFq6bzQqKXYqWy0Qx4amMNXqj8LcRMKjpftT8k=; b=gQ4+xsRBF9IWGRcCIzAeIDcbGUephS2iLgPWMRBSeX+u8zPv67eyl8q+PmPXy5VVjq hDZJe/vPO74oLf6UwA/YhXdHVtu4uQYoqIWlKNMpgOjWCPxaulOWxbf7UGG8ZAjVZ4xT J1+0BYpIGJzrSzEup4EzddZUFyiDF90NSvyfZmaET0CSwGKJaY11O9kxHlrCXmRHvcfv M8pZiXjIHqI4bKms27bkGgLuV1DUL5Ng9slpzl4tNVsIJ/8DGICTbtmDoDBy24abT60T k2cgJnzM+awssTAgbUK1xPYyPq4SJxou0giBj8OvevJs4WYaeS/E3Y4sZP8TDyv704GO Gi7Q== X-Gm-Message-State: ACgBeo228A5Sd8uZ+39lfZsouuAIfqsFyr2GH0QmiWjK9c0N3KDWez/i Wyi5d1Ac1d7NlBhP+PGf19eQLOb7sx+IPNvbwnk= X-Google-Smtp-Source: AA6agR62i/7z5gszNPxCHM2tUNT7IR0EnqVb391hZCNAMeUz4tSPO3zPDqa/NtagcRnkiNQJGORyj3nl76pLbZ0BL1M= X-Received: by 2002:ac8:7f92:0:b0:344:8cd8:59a1 with SMTP id z18-20020ac87f92000000b003448cd859a1mr25497299qtj.384.1662055465702; Thu, 01 Sep 2022 11:04:25 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Andy Shevchenko Date: Thu, 1 Sep 2022 21:03:49 +0300 Message-ID: Subject: Re: [PATCH 6/6] gpio: macsmc: Add IRQ support To: Russell King Cc: Arnd Bergmann , Lee Jones , Linus Walleij , Alyssa Rosenzweig , asahi@lists.linux.dev, Bartosz Golaszewski , Hector Martin , linux-arm Mailing List , "open list:GPIO SUBSYSTEM" , Sven Peter X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220901_110429_902966_4948300A X-CRM114-Status: GOOD ( 19.96 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, Sep 1, 2022 at 5:18 PM Russell King wrote: > > From: Hector Martin > > Add IRQ support to the macsmc driver. This patch has updates from Joey > Gouly and Russell King. ... > + u16 type = event >> 16; > + u8 offset = (event >> 8) & 0xff; The ' & 0xff' part is redundant. ... > + return (ret == 0) ? NOTIFY_OK : NOTIFY_DONE; Parentheses and ' == 0' parts are redundant. You may swap ternary operands. ... > +static void macsmc_gpio_irq_enable(struct irq_data *d) > +{ > + struct gpio_chip *gc = irq_data_get_irq_chip_data(d); > + struct macsmc_gpio *smcgp = gpiochip_get_data(gc); > + > + gpiochip_enable_irq(gc, irqd_to_hwirq(d)); > + set_bit(irqd_to_hwirq(d), smcgp->irq_enable_shadow); You may use temporary variable for hwirq irq_hw_number_t hwirq = irdq_to_hwirq(d); > +} > + > +static void macsmc_gpio_irq_disable(struct irq_data *d) > +{ > + struct gpio_chip *gc = irq_data_get_irq_chip_data(d); > + struct macsmc_gpio *smcgp = gpiochip_get_data(gc); > + > + clear_bit(irqd_to_hwirq(d), smcgp->irq_enable_shadow); > + gpiochip_disable_irq(gc, irqd_to_hwirq(d)); Ditto. > +} > + > +static int macsmc_gpio_irq_set_type(struct irq_data *d, unsigned int type) > +{ > + struct gpio_chip *gc = irq_data_get_irq_chip_data(d); > + struct macsmc_gpio *smcgp = gpiochip_get_data(gc); > + int offset = irqd_to_hwirq(d); As above. > + u32 mode; > + if (!test_bit(offset, smcgp->irq_supported)) > + return -EINVAL; We have a valid mask for IRQs. Can it be used here instead? > + switch (type & IRQ_TYPE_SENSE_MASK) { > + case IRQ_TYPE_LEVEL_HIGH: > + mode = IRQ_MODE_HIGH; > + break; > + case IRQ_TYPE_LEVEL_LOW: > + mode = IRQ_MODE_LOW; > + break; > + case IRQ_TYPE_EDGE_RISING: > + mode = IRQ_MODE_RISING; > + break; > + case IRQ_TYPE_EDGE_FALLING: > + mode = IRQ_MODE_FALLING; > + break; > + case IRQ_TYPE_EDGE_BOTH: > + mode = IRQ_MODE_BOTH; > + break; > + default: > + return -EINVAL; > } > > + smcgp->irq_mode_shadow[offset] = mode; Usually we want to have handle_bad_irq() handler by default and in ->set_type() we lock a handler depending on the flags. Why is this not the case in this driver? > return 0; > } ... > +static void macsmc_gpio_irq_bus_sync_unlock(struct irq_data *d) > +{ > + struct gpio_chip *gc = irq_data_get_irq_chip_data(d); > + struct macsmc_gpio *smcgp = gpiochip_get_data(gc); > + smc_key key = macsmc_gpio_key(irqd_to_hwirq(d)); > + int offset = irqd_to_hwirq(d); As above. > + bool val; > + > + if (smcgp->irq_mode_shadow[offset] != smcgp->irq_mode[offset]) { > + u32 cmd = CMD_IRQ_MODE | smcgp->irq_mode_shadow[offset]; > + if (apple_smc_write_u32(smcgp->smc, key, cmd) < 0) > + dev_err(smcgp->dev, "GPIO IRQ config failed for %p4ch = 0x%x\n", &key, cmd); > + else > + smcgp->irq_mode_shadow[offset] = smcgp->irq_mode[offset]; > + } > + > + val = test_bit(offset, smcgp->irq_enable_shadow); > + if (test_bit(offset, smcgp->irq_enable) != val) { > + if (apple_smc_write_u32(smcgp->smc, key, CMD_IRQ_ENABLE | val) < 0) > + dev_err(smcgp->dev, "GPIO IRQ en/disable failed for %p4ch\n", &key); > + else > + change_bit(offset, smcgp->irq_enable); > + } > + > + mutex_unlock(&smcgp->irq_mutex); > +} -- With Best Regards, Andy Shevchenko _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel