Message ID | 20230922224027.85291-2-matti.lehtimaki@gmail.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:612c:172:b0:3f2:4152:657d with SMTP id h50csp5907046vqi; Fri, 22 Sep 2023 15:42:54 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHifcf3A/r3N9oOCBavzfV480AFryQTUbnvbomYW1NbC8jR6a2gWSjrsk01vdnMTQ3Ovi1h X-Received: by 2002:a05:6808:208e:b0:3a8:4e27:3af3 with SMTP id s14-20020a056808208e00b003a84e273af3mr1195926oiw.48.1695422573746; Fri, 22 Sep 2023 15:42:53 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1695422573; cv=none; d=google.com; s=arc-20160816; b=yPsjjHrw7clq7D+j7w0CazklEykxYz7zs8i/XDWnbTvyVwOLl1uQzgyp9C4c7VqZef O/IJ7mVb44cv7oJpinax/YZXqCqbO5wSzcFbA+EcCufPGjV6/525ZncZaIqjXbHU7Gh9 k7a6clH1eNbTXxZT0ve2c6KOoTATbhvc8fE9VqTbVQIkvcWSKgpgvlCGDpwRmShfqUgW k7EMjIe4Zyy6KKofwpelBE7nA3vRn1WGiwUO4tJdLAJz3n53GnpulnFZdYBHVaECuB0k /JColzu1WMVXjJ4iHFPUY9VYs0bIfzcAuWJ9ldGHII6wFrcUUi8u0f7wVt63AIrSH/0r QFCw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=tnnYflUXki788FbyEpOYccXKNL0/Zg1ISD6i+gaFKJg=; fh=KK1u1CY3y3cF6IBMx+639trJcCNvL2tT/y3DoBRs8X0=; b=RG7aWc9+fHY0uwsGUWLevPVY7PjqUoLJHXQXlr9eq+PdU0QjZAZXYir65IekqZq7yv sGjE3sA2INZP/nvQO6ct/lTB5s5UfYRhm/KMMr5K+t+PaWkjLeq2xNz8ZYtQKlHfUm3M /AWLSeidxs5Z6npqSalNvuK56sgsMABrs/HJdvjXU/wgzTlwXJ7mB4CVj/lLZh9f/dH0 Oxov7WTR+g3JwNCXAd0orT6l1b2vMAo5paRv/GzYVaAzr4BgBsTpiXNOQfv/H+8l/+0T D7dNjoN9Luy4BE+plvQxdcQTrxRe5G4jIsM0qzIF4BcB2D24MdzuOaBRBMPUOvjzQP/X PoZg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=I5KGoDCf; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from pete.vger.email (pete.vger.email. [23.128.96.36]) by mx.google.com with ESMTPS id bw9-20020a056a00408900b0068fc49cc456si4753371pfb.248.2023.09.22.15.42.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 22 Sep 2023 15:42:53 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) client-ip=23.128.96.36; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=I5KGoDCf; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by pete.vger.email (Postfix) with ESMTP id DCFF6803D5B8; Fri, 22 Sep 2023 15:41:07 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at pete.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230136AbjIVWkm (ORCPT <rfc822;pwkd43@gmail.com> + 28 others); Fri, 22 Sep 2023 18:40:42 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59082 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230004AbjIVWkk (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 22 Sep 2023 18:40:40 -0400 Received: from mail-lj1-x230.google.com (mail-lj1-x230.google.com [IPv6:2a00:1450:4864:20::230]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3851519A; Fri, 22 Sep 2023 15:40:34 -0700 (PDT) Received: by mail-lj1-x230.google.com with SMTP id 38308e7fff4ca-2c02e232c48so49409711fa.1; Fri, 22 Sep 2023 15:40:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1695422432; x=1696027232; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=tnnYflUXki788FbyEpOYccXKNL0/Zg1ISD6i+gaFKJg=; b=I5KGoDCfhg2FLuq+5+qoq+3u03alPW0wUGN+Pwsz0ngEu9r1jRZ9T9WHZWyBw9GRVt ylXaMU/vIDGU0ATEnXG67cc4WvnydjBfK8XQh1JiIMgWtF3lXbtHsKbZd3jtq1B+RA6c VxAZuRjfdB3pZkCF68YLil09EDT+0fsSR/mmyo2C6bkUzl19fNDVPr8gRsWY9RElu4ov 5veZu+ketPy1TNJrANI4QSIHfhDcnaXwLnCO5DmXPK1tTJ1iYTo/Hrux5yi7SlBoSe6o OCtQwX1ArLjP4N/KXmxp1KV7PbGEPXjkQQ8fE+X/u/a5u4I7S5onY43aEyr1SbgWFM+P ormg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695422432; x=1696027232; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=tnnYflUXki788FbyEpOYccXKNL0/Zg1ISD6i+gaFKJg=; b=uAR8V083gMRjMSivi02g1T0V+HNBNjFNxcPPWG42xTtruMlG1hvJ01O0hceQ1vzxGI x83IdTrakTYLIanOc7GIh+GOCsSBoceARKcMRyhkU3Cq7jYacXtq5DtI/PmxS8WPSX4h KV+UFrrcant2u758fXre1DmNk6kOeqPKoFJQOp+5j9RFfCpP9aQxY7hKKQCNRhBEkZU8 dnL0z78z1bAh7JahFtQsfp0/oP5cqPBeQKeLE8Pg5WRyiJa/MaFLfLHzm6WKFY4u5klr sW5V5AuCRE62qsc88XH9zXOEnItSxQtlpMRUogILENiOGJZ6Tgatdj6KQTNfp+sc8yPl 9kLQ== X-Gm-Message-State: AOJu0YytCIovwBS2v+nkQhx1EDOJ4q22hxBmFIZqpCaqbEhCGJvaLHy0 PpoQKmAi0L1uJyIf1M7qMfw+U4J2L0YJsA== X-Received: by 2002:a2e:b051:0:b0:2bc:f252:6cc4 with SMTP id d17-20020a2eb051000000b002bcf2526cc4mr401876ljl.10.1695422432122; Fri, 22 Sep 2023 15:40:32 -0700 (PDT) Received: from i-vetokaappi.home.lan (dsl-hkibng42-56733b-36.dhcp.inet.fi. [86.115.59.36]) by smtp.gmail.com with ESMTPSA id j2-20020a2e8502000000b002bfec05a693sm1090423lji.22.2023.09.22.15.40.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 22 Sep 2023 15:40:31 -0700 (PDT) From: =?utf-8?q?Matti_Lehtim=C3=A4ki?= <matti.lehtimaki@gmail.com> To: linux-arm-msm@vger.kernel.org Cc: ~postmarketos/upstreaming@lists.sr.ht, phone-devel@vger.kernel.org, =?utf-8?q?Matti_Lehtim=C3=A4ki?= <matti.lehtimaki@gmail.com>, Bjorn Andersson <andersson@kernel.org>, Andy Gross <agross@kernel.org>, Konrad Dybcio <konrad.dybcio@linaro.org>, Linus Walleij <linus.walleij@linaro.org>, linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH 1/2] pinctrl: qcom: msm8226: Add MPM pin mappings Date: Sat, 23 Sep 2023 01:40:26 +0300 Message-Id: <20230922224027.85291-2-matti.lehtimaki@gmail.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20230922224027.85291-1-matti.lehtimaki@gmail.com> References: <20230922224027.85291-1-matti.lehtimaki@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=3.0 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_SBL_CSS, SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on pete.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (pete.vger.email [0.0.0.0]); Fri, 22 Sep 2023 15:41:07 -0700 (PDT) X-Spam-Level: ** X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1777779420712248671 X-GMAIL-MSGID: 1777779420712248671 |
Series |
MPM pin mappings for MSM8226 and MSM8974
|
|
Commit Message
Matti Lehtimäki
Sept. 22, 2023, 10:40 p.m. UTC
Add pin <-> wakeirq mappings to allow for waking up the AP from sleep
through MPM-connected pins.
Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
---
drivers/pinctrl/qcom/pinctrl-msm8226.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
Comments
Hi Matti, On Samstag, 23. September 2023 00:40:26 CEST Matti Lehtimäki wrote: > Add pin <-> wakeirq mappings to allow for waking up the AP from sleep > through MPM-connected pins. > > Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com> > --- > drivers/pinctrl/qcom/pinctrl-msm8226.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/pinctrl/qcom/pinctrl-msm8226.c > b/drivers/pinctrl/qcom/pinctrl-msm8226.c index 994619840a70..1e46a9ab382f > 100644 > --- a/drivers/pinctrl/qcom/pinctrl-msm8226.c > +++ b/drivers/pinctrl/qcom/pinctrl-msm8226.c > @@ -612,6 +612,16 @@ static const struct msm_pingroup msm8226_groups[] = { > > #define NUM_GPIO_PINGROUPS 117 > > +static const struct msm_gpio_wakeirq_map msm8226_mpm_map[] = { > + { 1, 3 }, { 4, 4 }, { 5, 5 }, { 9, 6 }, { 13, 7 }, { 17, 8 }, I'm not really convinced this is the correct order of values... Let's look at downstream: qcom,gpio-map = <3 1>, <4 4 >, <5 5 >, <6 9 >, [...] From Documentation/devicetree/bindings/arm/msm/mpm.txt downstream: Each tuple represents a MPM pin and which GIC interrupt is routed to it. So first is pin number, second is interrupt number. And check mainline: /** * struct msm_gpio_wakeirq_map - Map of GPIOs and their wakeup pins * @gpio: The GPIOs that are wakeup capable * @wakeirq: The interrupt at the always-on interrupt controller */ struct msm_gpio_wakeirq_map { unsigned int gpio; unsigned int wakeirq; }; So here we also have the order pin-interrupt, not the reverse order. Therefore I believe the order in this patch is incorrect, and it should rather be: { 3, 1 }, { 4, 4 }, { 5, 5 }, { 6, 9 }, { 7, 13 }, { 8, 17 }, [...] Or do you think I'm missing something? Regards Luca > + { 21, 9 }, { 27, 10 }, { 29, 11 }, { 31, 12 }, { 33, 13 }, { 35, 14 }, > + { 37, 15 }, { 38, 16 }, { 39, 17 }, { 41, 18 }, { 46, 19 }, { 48, 20 }, > + { 49, 21 }, { 50, 22 }, { 51, 23 }, { 52, 24 }, { 54, 25 }, { 62, 26 }, > + { 63, 27 }, { 64, 28 }, { 65, 29 }, { 66, 30 }, { 67, 31 }, { 68, 32 }, > + { 69, 33 }, { 71, 34 }, { 72, 35 }, { 106, 36 }, { 107, 37 }, > + { 108, 38 }, { 109, 39 }, { 110, 40 }, { 111, 54 }, { 113, 55 }, > +}; > + > static const struct msm_pinctrl_soc_data msm8226_pinctrl = { > .pins = msm8226_pins, > .npins = ARRAY_SIZE(msm8226_pins), > @@ -620,6 +630,8 @@ static const struct msm_pinctrl_soc_data msm8226_pinctrl > = { .groups = msm8226_groups, > .ngroups = ARRAY_SIZE(msm8226_groups), > .ngpios = NUM_GPIO_PINGROUPS, > + .wakeirq_map = msm8226_mpm_map, > + .nwakeirq_map = ARRAY_SIZE(msm8226_mpm_map), > }; > > static int msm8226_pinctrl_probe(struct platform_device *pdev)
On Sat, Sep 23, 2023 at 11:32:47AM +0200, Luca Weiss wrote: > Hi Matti, > > On Samstag, 23. September 2023 00:40:26 CEST Matti Lehtimäki wrote: > > Add pin <-> wakeirq mappings to allow for waking up the AP from sleep > > through MPM-connected pins. > > > > Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com> > > --- > > drivers/pinctrl/qcom/pinctrl-msm8226.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/drivers/pinctrl/qcom/pinctrl-msm8226.c > > b/drivers/pinctrl/qcom/pinctrl-msm8226.c index 994619840a70..1e46a9ab382f > > 100644 > > --- a/drivers/pinctrl/qcom/pinctrl-msm8226.c > > +++ b/drivers/pinctrl/qcom/pinctrl-msm8226.c > > @@ -612,6 +612,16 @@ static const struct msm_pingroup msm8226_groups[] = { > > > > #define NUM_GPIO_PINGROUPS 117 > > > > +static const struct msm_gpio_wakeirq_map msm8226_mpm_map[] = { > > + { 1, 3 }, { 4, 4 }, { 5, 5 }, { 9, 6 }, { 13, 7 }, { 17, 8 }, > > I'm not really convinced this is the correct order of values... > > Let's look at downstream: > > qcom,gpio-map = <3 1>, > <4 4 >, > <5 5 >, > <6 9 >, > [...] > > From Documentation/devicetree/bindings/arm/msm/mpm.txt downstream: > > Each tuple represents a MPM pin and which GIC interrupt is routed to it. > > So first is pin number, second is interrupt number. > > And check mainline: > > /** > * struct msm_gpio_wakeirq_map - Map of GPIOs and their wakeup pins > * @gpio: The GPIOs that are wakeup capable > * @wakeirq: The interrupt at the always-on interrupt controller > */ > struct msm_gpio_wakeirq_map { > unsigned int gpio; > unsigned int wakeirq; > }; > > So here we also have the order pin-interrupt, not the reverse order. > > Therefore I believe the order in this patch is incorrect, and it should rather > be: > > { 3, 1 }, { 4, 4 }, { 5, 5 }, { 6, 9 }, { 7, 13 }, { 8, 17 }, > [...] > > Or do you think I'm missing something? > Yes :) Let's look at the later entries: > > + { 21, 9 }, { 27, 10 }, { 29, 11 }, { 31, 12 }, { 33, 13 }, { 35, 14 > }, > > + { 37, 15 }, { 38, 16 }, { 39, 17 }, { 41, 18 }, { 46, 19 }, { 48, 20 > }, > > + { 49, 21 }, { 50, 22 }, { 51, 23 }, { 52, 24 }, { 54, 25 }, { 62, 26 > }, > > + { 63, 27 }, { 64, 28 }, { 65, 29 }, { 66, 30 }, { 67, 31 }, { 68, 32 > }, > > + { 69, 33 }, { 71, 34 }, { 72, 35 }, { 106, 36 }, { 107, 37 }, > > + { 108, 38 }, { 109, 39 }, { 110, 40 }, { 111, 54 }, { 113, 55 }, > > +}; > > + For example: { 113, 55 }, i.e. { .gpio = 113, .wakeirq = 55 }. MSM8226 has GPIOs 0-116 and 64 MPM pins/interrupts. The order in this patch is the only one that can be correct because the definition would be invalid the other way around. 113 must be the GPIO number because it is larger than the 64 available MPM interrupt pins. :) Thanks, Stephan
On Samstag, 23. September 2023 12:00:52 CEST Stephan Gerhold wrote: > On Sat, Sep 23, 2023 at 11:32:47AM +0200, Luca Weiss wrote: > > Hi Matti, > > > > On Samstag, 23. September 2023 00:40:26 CEST Matti Lehtimäki wrote: > > > Add pin <-> wakeirq mappings to allow for waking up the AP from sleep > > > through MPM-connected pins. > > > > > > Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com> > > > --- > > > > > > drivers/pinctrl/qcom/pinctrl-msm8226.c | 12 ++++++++++++ > > > 1 file changed, 12 insertions(+) > > > > > > diff --git a/drivers/pinctrl/qcom/pinctrl-msm8226.c > > > b/drivers/pinctrl/qcom/pinctrl-msm8226.c index > > > 994619840a70..1e46a9ab382f > > > 100644 > > > --- a/drivers/pinctrl/qcom/pinctrl-msm8226.c > > > +++ b/drivers/pinctrl/qcom/pinctrl-msm8226.c > > > @@ -612,6 +612,16 @@ static const struct msm_pingroup msm8226_groups[] = > > > { > > > > > > #define NUM_GPIO_PINGROUPS 117 > > > > > > +static const struct msm_gpio_wakeirq_map msm8226_mpm_map[] = { > > > + { 1, 3 }, { 4, 4 }, { 5, 5 }, { 9, 6 }, { 13, 7 }, { 17, 8 }, > > > > I'm not really convinced this is the correct order of values... > > > > Let's look at downstream: > > qcom,gpio-map = <3 1>, > > > > <4 4 >, > > <5 5 >, > > <6 9 >, > > [...] > > > > From Documentation/devicetree/bindings/arm/msm/mpm.txt downstream: > > Each tuple represents a MPM pin and which GIC interrupt is routed to it. > > > > So first is pin number, second is interrupt number. > > > > And check mainline: > > /** > > > > * struct msm_gpio_wakeirq_map - Map of GPIOs and their wakeup pins > > * @gpio: The GPIOs that are wakeup capable > > * @wakeirq: The interrupt at the always-on interrupt controller > > */ > > > > struct msm_gpio_wakeirq_map { > > > > unsigned int gpio; > > unsigned int wakeirq; > > > > }; > > > > So here we also have the order pin-interrupt, not the reverse order. > > > > Therefore I believe the order in this patch is incorrect, and it should > > rather> > > be: > > { 3, 1 }, { 4, 4 }, { 5, 5 }, { 6, 9 }, { 7, 13 }, { 8, 17 }, > > [...] > > > > Or do you think I'm missing something? > > Yes :) > > Let's look at the later entries: > > > + { 21, 9 }, { 27, 10 }, { 29, 11 }, { 31, 12 }, { 33, 13 }, { 35, 14 > > > > }, > > > > > + { 37, 15 }, { 38, 16 }, { 39, 17 }, { 41, 18 }, { 46, 19 }, { 48, 20 > > > > }, > > > > > + { 49, 21 }, { 50, 22 }, { 51, 23 }, { 52, 24 }, { 54, 25 }, { 62, 26 > > > > }, > > > > > + { 63, 27 }, { 64, 28 }, { 65, 29 }, { 66, 30 }, { 67, 31 }, { 68, 32 > > > > }, > > > > > + { 69, 33 }, { 71, 34 }, { 72, 35 }, { 106, 36 }, { 107, 37 }, > > > + { 108, 38 }, { 109, 39 }, { 110, 40 }, { 111, 54 }, { 113, 55 }, > > > +}; > > > + > > For example: { 113, 55 }, i.e. { .gpio = 113, .wakeirq = 55 }. > > MSM8226 has GPIOs 0-116 and 64 MPM pins/interrupts. The order in this > patch is the only one that can be correct because the definition would > be invalid the other way around. 113 must be the GPIO number because it > is larger than the 64 available MPM interrupt pins. :) So basically you're saying downstream is wrong / buggy? From qcom,gpio-map = [...], <55 113>; it's taking the properties like this (drivers/soc/qcom/mpm-of.c): unsigned long pin = be32_to_cpup(list++); irq_hw_number_t hwirq = be32_to_cpup(list++); Your explanation does make sense I guess but somewhere the link downstream -> mainline must be broken, no? Regards Luca > > Thanks, > Stephan
On Sat, Sep 23, 2023 at 01:19:46PM +0200, Luca Weiss wrote: > On Samstag, 23. September 2023 12:00:52 CEST Stephan Gerhold wrote: > > On Sat, Sep 23, 2023 at 11:32:47AM +0200, Luca Weiss wrote: > > > Hi Matti, > > > > > > On Samstag, 23. September 2023 00:40:26 CEST Matti Lehtimäki wrote: > > > > Add pin <-> wakeirq mappings to allow for waking up the AP from sleep > > > > through MPM-connected pins. > > > > > > > > Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com> > > > > --- > > > > > > > > drivers/pinctrl/qcom/pinctrl-msm8226.c | 12 ++++++++++++ > > > > 1 file changed, 12 insertions(+) > > > > > > > > diff --git a/drivers/pinctrl/qcom/pinctrl-msm8226.c > > > > b/drivers/pinctrl/qcom/pinctrl-msm8226.c index > > > > 994619840a70..1e46a9ab382f > > > > 100644 > > > > --- a/drivers/pinctrl/qcom/pinctrl-msm8226.c > > > > +++ b/drivers/pinctrl/qcom/pinctrl-msm8226.c > > > > @@ -612,6 +612,16 @@ static const struct msm_pingroup msm8226_groups[] = > > > > { > > > > > > > > #define NUM_GPIO_PINGROUPS 117 > > > > > > > > +static const struct msm_gpio_wakeirq_map msm8226_mpm_map[] = { > > > > + { 1, 3 }, { 4, 4 }, { 5, 5 }, { 9, 6 }, { 13, 7 }, { 17, 8 }, > > > > > > I'm not really convinced this is the correct order of values... > > > > > > Let's look at downstream: > > > qcom,gpio-map = <3 1>, > > > > > > <4 4 >, > > > <5 5 >, > > > <6 9 >, > > > [...] > > > > > > From Documentation/devicetree/bindings/arm/msm/mpm.txt downstream: > > > Each tuple represents a MPM pin and which GIC interrupt is routed to it. > > > > > > So first is pin number, second is interrupt number. > > > > > > And check mainline: > > > /** > > > > > > * struct msm_gpio_wakeirq_map - Map of GPIOs and their wakeup pins > > > * @gpio: The GPIOs that are wakeup capable > > > * @wakeirq: The interrupt at the always-on interrupt controller > > > */ > > > > > > struct msm_gpio_wakeirq_map { > > > > > > unsigned int gpio; > > > unsigned int wakeirq; > > > > > > }; > > > > > > So here we also have the order pin-interrupt, not the reverse order. > > > > > > Therefore I believe the order in this patch is incorrect, and it should > > > rather> > > > be: > > > { 3, 1 }, { 4, 4 }, { 5, 5 }, { 6, 9 }, { 7, 13 }, { 8, 17 }, > > > [...] > > > > > > Or do you think I'm missing something? > > > > Yes :) > > > > Let's look at the later entries: > > > > + { 21, 9 }, { 27, 10 }, { 29, 11 }, { 31, 12 }, { 33, 13 }, { 35, 14 > > > > > > }, > > > > > > > + { 37, 15 }, { 38, 16 }, { 39, 17 }, { 41, 18 }, { 46, 19 }, { 48, 20 > > > > > > }, > > > > > > > + { 49, 21 }, { 50, 22 }, { 51, 23 }, { 52, 24 }, { 54, 25 }, { 62, 26 > > > > > > }, > > > > > > > + { 63, 27 }, { 64, 28 }, { 65, 29 }, { 66, 30 }, { 67, 31 }, { 68, 32 > > > > > > }, > > > > > > > + { 69, 33 }, { 71, 34 }, { 72, 35 }, { 106, 36 }, { 107, 37 }, > > > > + { 108, 38 }, { 109, 39 }, { 110, 40 }, { 111, 54 }, { 113, 55 }, > > > > +}; > > > > + > > > > For example: { 113, 55 }, i.e. { .gpio = 113, .wakeirq = 55 }. > > > > MSM8226 has GPIOs 0-116 and 64 MPM pins/interrupts. The order in this > > patch is the only one that can be correct because the definition would > > be invalid the other way around. 113 must be the GPIO number because it > > is larger than the 64 available MPM interrupt pins. :) > > So basically you're saying downstream is wrong / buggy? > "Misleading" or "confusing" would be the words I would use. :-) > From qcom,gpio-map = [...], <55 113>; it's taking the properties like this > (drivers/soc/qcom/mpm-of.c): > > unsigned long pin = be32_to_cpup(list++); > irq_hw_number_t hwirq = be32_to_cpup(list++); > > Your explanation does make sense I guess but somewhere the link downstream -> > mainline must be broken, no? > After staring at mpm-of.c for a while I would say that there: - downstream "pin" = MPM pin = mainline "wakeirq" - because this is used as index to msm_mpm_irqs_m2a, which has a size of MSM_MPM_NR_MPM_IRQS (64) - downstream "hwirq" = GPIO / GIC IRQ = mainline "gpio" This means for <55 113>: pin = wakeirq = 55 and hwirq = gpio = 113. Which matches the definition in this patch: { .gpio = 113, .wakeirq = 55 } = { 113, 55 } Stephan
On Samstag, 23. September 2023 13:35:25 CEST Stephan Gerhold wrote: > On Sat, Sep 23, 2023 at 01:19:46PM +0200, Luca Weiss wrote: > > On Samstag, 23. September 2023 12:00:52 CEST Stephan Gerhold wrote: > > > On Sat, Sep 23, 2023 at 11:32:47AM +0200, Luca Weiss wrote: > > > > Hi Matti, > > > > > > > > On Samstag, 23. September 2023 00:40:26 CEST Matti Lehtimäki wrote: > > > > > Add pin <-> wakeirq mappings to allow for waking up the AP from > > > > > sleep > > > > > through MPM-connected pins. > > > > > > > > > > Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com> > > > > > --- > > > > > > > > > > drivers/pinctrl/qcom/pinctrl-msm8226.c | 12 ++++++++++++ > > > > > 1 file changed, 12 insertions(+) > > > > > > > > > > diff --git a/drivers/pinctrl/qcom/pinctrl-msm8226.c > > > > > b/drivers/pinctrl/qcom/pinctrl-msm8226.c index > > > > > 994619840a70..1e46a9ab382f > > > > > 100644 > > > > > --- a/drivers/pinctrl/qcom/pinctrl-msm8226.c > > > > > +++ b/drivers/pinctrl/qcom/pinctrl-msm8226.c > > > > > @@ -612,6 +612,16 @@ static const struct msm_pingroup > > > > > msm8226_groups[] = > > > > > { > > > > > > > > > > #define NUM_GPIO_PINGROUPS 117 > > > > > > > > > > +static const struct msm_gpio_wakeirq_map msm8226_mpm_map[] = { > > > > > + { 1, 3 }, { 4, 4 }, { 5, 5 }, { 9, 6 }, { 13, 7 }, { 17, 8 }, > > > > > > > > I'm not really convinced this is the correct order of values... > > > > > > > > Let's look at downstream: > > > > qcom,gpio-map = <3 1>, > > > > > > > > <4 4 >, > > > > <5 5 >, > > > > <6 9 >, > > > > [...] > > > > > > > > From Documentation/devicetree/bindings/arm/msm/mpm.txt downstream: > > > > Each tuple represents a MPM pin and which GIC interrupt is routed to > > > > it. > > > > > > > > So first is pin number, second is interrupt number. > > > > > > > > And check mainline: > > > > /** > > > > > > > > * struct msm_gpio_wakeirq_map - Map of GPIOs and their wakeup pins > > > > * @gpio: The GPIOs that are wakeup capable > > > > * @wakeirq: The interrupt at the always-on interrupt > > > > controller > > > > */ > > > > > > > > struct msm_gpio_wakeirq_map { > > > > > > > > unsigned int gpio; > > > > unsigned int wakeirq; > > > > > > > > }; > > > > > > > > So here we also have the order pin-interrupt, not the reverse order. > > > > > > > > Therefore I believe the order in this patch is incorrect, and it > > > > should > > > > rather> > > > > > > > > be: > > > > { 3, 1 }, { 4, 4 }, { 5, 5 }, { 6, 9 }, { 7, 13 }, { 8, 17 }, > > > > [...] > > > > > > > > Or do you think I'm missing something? > > > > > > Yes :) > > > > > > Let's look at the later entries: > > > > > + { 21, 9 }, { 27, 10 }, { 29, 11 }, { 31, 12 }, { 33, 13 }, { 35, > > > > > 14 > > > > > > > > }, > > > > > > > > > + { 37, 15 }, { 38, 16 }, { 39, 17 }, { 41, 18 }, { 46, 19 }, { 48, > > > > > 20 > > > > > > > > }, > > > > > > > > > + { 49, 21 }, { 50, 22 }, { 51, 23 }, { 52, 24 }, { 54, 25 }, { 62, > > > > > 26 > > > > > > > > }, > > > > > > > > > + { 63, 27 }, { 64, 28 }, { 65, 29 }, { 66, 30 }, { 67, 31 }, { 68, > > > > > 32 > > > > > > > > }, > > > > > > > > > + { 69, 33 }, { 71, 34 }, { 72, 35 }, { 106, 36 }, { 107, 37 }, > > > > > + { 108, 38 }, { 109, 39 }, { 110, 40 }, { 111, 54 }, { 113, 55 }, > > > > > +}; > > > > > + > > > > > > For example: { 113, 55 }, i.e. { .gpio = 113, .wakeirq = 55 }. > > > > > > MSM8226 has GPIOs 0-116 and 64 MPM pins/interrupts. The order in this > > > patch is the only one that can be correct because the definition would > > > be invalid the other way around. 113 must be the GPIO number because it > > > is larger than the 64 available MPM interrupt pins. :) > > > > So basically you're saying downstream is wrong / buggy? > > "Misleading" or "confusing" would be the words I would use. :-) ;) > > > From qcom,gpio-map = [...], <55 113>; it's taking the properties like this > > > > (drivers/soc/qcom/mpm-of.c): > > unsigned long pin = be32_to_cpup(list++); > > irq_hw_number_t hwirq = be32_to_cpup(list++); > > > > Your explanation does make sense I guess but somewhere the link downstream > > -> mainline must be broken, no? > > After staring at mpm-of.c for a while I would say that there: > - downstream "pin" = MPM pin = mainline "wakeirq" > - because this is used as index to msm_mpm_irqs_m2a, which has a size > of MSM_MPM_NR_MPM_IRQS (64) > - downstream "hwirq" = GPIO / GIC IRQ = mainline "gpio" > > This means for <55 113>: pin = wakeirq = 55 and hwirq = gpio = 113. > Which matches the definition in this patch: > { .gpio = 113, .wakeirq = 55 } = { 113, 55 } Fun, thanks for digging into it! @Matti: I think I see one missing entry here "<41 115>," on downstream, so { 115, 41 } appears to be missing in this patch? Or is there a reason you omitted that one? The rest looks correct :) Regards Luca > > Stephan
diff --git a/drivers/pinctrl/qcom/pinctrl-msm8226.c b/drivers/pinctrl/qcom/pinctrl-msm8226.c index 994619840a70..1e46a9ab382f 100644 --- a/drivers/pinctrl/qcom/pinctrl-msm8226.c +++ b/drivers/pinctrl/qcom/pinctrl-msm8226.c @@ -612,6 +612,16 @@ static const struct msm_pingroup msm8226_groups[] = { #define NUM_GPIO_PINGROUPS 117 +static const struct msm_gpio_wakeirq_map msm8226_mpm_map[] = { + { 1, 3 }, { 4, 4 }, { 5, 5 }, { 9, 6 }, { 13, 7 }, { 17, 8 }, + { 21, 9 }, { 27, 10 }, { 29, 11 }, { 31, 12 }, { 33, 13 }, { 35, 14 }, + { 37, 15 }, { 38, 16 }, { 39, 17 }, { 41, 18 }, { 46, 19 }, { 48, 20 }, + { 49, 21 }, { 50, 22 }, { 51, 23 }, { 52, 24 }, { 54, 25 }, { 62, 26 }, + { 63, 27 }, { 64, 28 }, { 65, 29 }, { 66, 30 }, { 67, 31 }, { 68, 32 }, + { 69, 33 }, { 71, 34 }, { 72, 35 }, { 106, 36 }, { 107, 37 }, + { 108, 38 }, { 109, 39 }, { 110, 40 }, { 111, 54 }, { 113, 55 }, +}; + static const struct msm_pinctrl_soc_data msm8226_pinctrl = { .pins = msm8226_pins, .npins = ARRAY_SIZE(msm8226_pins), @@ -620,6 +630,8 @@ static const struct msm_pinctrl_soc_data msm8226_pinctrl = { .groups = msm8226_groups, .ngroups = ARRAY_SIZE(msm8226_groups), .ngpios = NUM_GPIO_PINGROUPS, + .wakeirq_map = msm8226_mpm_map, + .nwakeirq_map = ARRAY_SIZE(msm8226_mpm_map), }; static int msm8226_pinctrl_probe(struct platform_device *pdev)