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=-15.4 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_2 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 E9E1BC433E0 for ; Thu, 21 Jan 2021 15:57:20 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 9049A238A0 for ; Thu, 21 Jan 2021 15:57:20 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9049A238A0 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=4a4sFxNOyEb1CKEbOQSqt4HuyY3dN1YsgoWGO2PYB4E=; b=sSFwewMmpCZPcdHCTrT9bt14X L4DbOYLcBXBEepORyCE1rTxdTJCubaQwoohImbW15l4vgVeEYZWPcvkP+X3HVhT3LNlds+6XICq/o NX88EZdVyRtfLDmVkSS0r2s/wWgjtyj1Yl0kZhKqycBtj97BMYY776FVsV9GoOiaw5h3iNCfhwslt rxTLwQf4NRxkzEImbkGA/Up2WWr7HHyq4g9Qe61D636v4Lnl/DQdZs0IHfguK4Ss+OSEY35oL8gVX M7sexFdn4sx4X2x48Z7TVoUhILCTklQyc/GrmCtvF918pjgD7oVqyxBY1guxRxNdAwSRwuj/gRw74 9wzBPCa0w==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1l2cIu-00048G-4x; Thu, 21 Jan 2021 15:55:24 +0000 Received: from foss.arm.com ([217.140.110.172]) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1l2cIq-000476-Ki for linux-arm-kernel@lists.infradead.org; Thu, 21 Jan 2021 15:55:21 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id E5ED411D4; Thu, 21 Jan 2021 07:55:16 -0800 (PST) Received: from slackpad.fritz.box (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id F305D3F68F; Thu, 21 Jan 2021 07:55:14 -0800 (PST) Date: Thu, 21 Jan 2021 15:54:39 +0000 From: Andre Przywara To: Samuel Holland Subject: Re: [PATCH v3 09/21] mfd: axp20x: Allow AXP chips without interrupt lines Message-ID: <20210121155439.79f4051c@slackpad.fritz.box> In-Reply-To: References: <20210118020848.11721-1-andre.przywara@arm.com> <20210118020848.11721-10-andre.przywara@arm.com> Organization: Arm Ltd. X-Mailer: Claws Mail 3.17.1 (GTK+ 2.24.31; x86_64-slackware-linux-gnu) MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210121_105520_764204_26355E7F X-CRM114-Status: GOOD ( 32.87 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Jernej Skrabec , Yangtao Li , Linus Walleij , linux-sunxi@googlegroups.com, linux-kernel@vger.kernel.org, Maxime Ripard , Chen-Yu Tsai , =?UTF-8?B?Q2zDqW1lbnQgUMOpcm9u?= , Shuosheng Huang , Lee Jones , linux-arm-kernel@lists.infradead.org, Icenowy Zheng 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 Sun, 17 Jan 2021 21:37:22 -0600 Samuel Holland wrote: Hi Samuel, thanks for your input! > On 1/17/21 8:08 PM, Andre Przywara wrote: > > Currently the AXP chip requires to have its IRQ line connected to some > > interrupt controller, and will fail probing when this is not the case. > > > > On a new Allwinner SoC (H616) there is no NMI pin anymore, so the > > interrupt functionality of the AXP chip is simply not available. > > > > Check whether the DT describes the AXP chip as an interrupt controller > > before trying to register the irqchip, to avoid probe failures on > > setups without an interrupt. > > The AXP305 has an IRQ pin. It is still an interrupt controller, even if > its output is not connected anywhere. And even though the NMI pin on the > H616 is gone, the PMIC IRQ line could be connected to a GPIO. So it is > not appropriate to remove "interrupt-controller". That's a fair point. > Per the binding, both "interrupts" and "interrupt-controller" are > required properties. It would make more sense to make "interrupts" > optional. Either way, you need to update the binding. I agree. So I will replace the explicit check for the interrupt-controller property with a check for axp20x->irq being not 0 (which is apparently the right check for this, according to my research). And also adjust the binding to make "interrupts" optional. > Though I'm concerned about how this may affect drivers for regmap cells > which use interrupts (such as axp20x-pek). If the irqchip is not > registered, requesting those interrupts will fail. While I don't > currently know of any boards that have the AXP305 power key wired up, it > prevents us from modelling the hardware correctly and supporting that > configuration. Good point! Indeed axp20x_pek_probe() crashes with a NULL pointer dereference. I think this device is unconditionally tied to the AXP drivers, and this is probably fine, as it looks trivial to check the regmap_irqc pointer before passing it on to regmap_irq_get_virq(), bailing out if this is NULL. Will send the patch shortly, then update this patch here as well. And I guess the outcome (power button input device not available) is reasonable as well. The hardware power button feature (off after 6s) would work nevertheless. If board vendors expect more functionality from the button, they should connect the AXP IRQ pin to a GPIO. Cheers, Andre > > Cheers, > Samuel > > > Signed-off-by: Andre Przywara > > --- > > drivers/mfd/axp20x.c | 17 +++++++++++------ > > 1 file changed, 11 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c > > index aa59496e4376..a52595c49d40 100644 > > --- a/drivers/mfd/axp20x.c > > +++ b/drivers/mfd/axp20x.c > > @@ -959,12 +959,17 @@ int axp20x_device_probe(struct axp20x_dev *axp20x) > > AXP806_REG_ADDR_EXT_ADDR_SLAVE_MODE); > > } > > > > - ret = regmap_add_irq_chip(axp20x->regmap, axp20x->irq, > > - IRQF_ONESHOT | IRQF_SHARED | axp20x->irq_flags, > > - -1, axp20x->regmap_irq_chip, &axp20x->regmap_irqc); > > - if (ret) { > > - dev_err(axp20x->dev, "failed to add irq chip: %d\n", ret); > > - return ret; > > + if (!axp20x->dev->of_node || > > + of_property_read_bool(axp20x->dev->of_node, "interrupt-controller")) { > > + ret = regmap_add_irq_chip(axp20x->regmap, axp20x->irq, > > + IRQF_ONESHOT | IRQF_SHARED | axp20x->irq_flags, > > + -1, axp20x->regmap_irq_chip, > > + &axp20x->regmap_irqc); > > + if (ret) { > > + dev_err(axp20x->dev, "failed to add irq chip: %d\n", > > + ret); > > + return ret; > > + } > > } > > > > ret = mfd_add_devices(axp20x->dev, -1, axp20x->cells, > > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel