Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 26 Oct 2022 14:20:11 +0800
From:   Peng Fan <peng.fan@nxp.com>
To:     Shenwei Wang <shenwei.wang@nxp.com>
Cc:     Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Shawn Guo <shawnguo@kernel.org>,
        Sascha Hauer <s.hauer@pengutronix.de>,
        Linus Walleij <linus.walleij@linaro.org>,
        Bartosz Golaszewski <brgl@bgdev.pl>,
        Dong Aisheng <aisheng.dong@nxp.com>,
        Jacky Bai <ping.bai@nxp.com>,
        Pengutronix Kernel Team <kernel@pengutronix.de>,
        Fabio Estevam <festevam@gmail.com>,
        NXP Linux Team <linux-imx@nxp.com>,
        devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org, imx@lists.linux.dev
Subject: Re: [PATCH v3 5/5] gpio: mxc: enable pad wakeup on i.MX8x platforms
Message-ID: <20221026062011.GA18160@linux-1xn6>
References: <20221025183244.615318-1-shenwei.wang@nxp.com>
 <20221025183244.615318-6-shenwei.wang@nxp.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20221025183244.615318-6-shenwei.wang@nxp.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?if3sKptwQDo7tLiGNLkLuBGkNLyWQg2lfEpV757VLdh232XyqNPQrqP3e3mQ?=
 =?us-ascii?Q?YzPAoRvBiUVg80++rYz/VnBo8249ucML8k8Ib8yEtNGbANUDmXwhTwJJHsPX?=
 =?us-ascii?Q?l5+lI6ZbKgDUA2hNroY5jNaEdpjOebZoKgBtP+AH+fKUcH8iT+EyfpDxj6Mv?=
 =?us-ascii?Q?ZxUeIxPrJywIiGpv1DzEhIlb5dSq9nCjuqtg3pmGAB5uj+OfIm35Cvp7lEAd?=
 =?us-ascii?Q?9sWssPVrHK5zObkUx4NvP70EB8HUTPplVIDRLB0gRWyFEG9J+0neWEfg7YQ+?=
 =?us-ascii?Q?y4a6KXl5w2ZjijCDuQ5jmDp+56enWzI7fSYsZ5ZZ2ChFl9v3CecnwoMt/Umk?=
 =?us-ascii?Q?mVqL9Wxg+PU4A1EwW6u261YNIb20BsnZaDNGgKMEdhP0bMDQKIVmzcroFsS/?=
 =?us-ascii?Q?GsCw2D74S6ZWH5rvHCtYKlSOZ0+DQVLhWyR8NRty8vtS2cXXhZOop89ygvnL?=
 =?us-ascii?Q?knnIHsG6UWIGvqYs0s/9iNCD/g8yMv7L8/mujSY9dp00FAB8PglFF6xtnL21?=
 =?us-ascii?Q?VMhThve40GhNCFyAtW3n9c+sFD5mR1SrmFUtRiH6dTN5n8krFq4lgLPv95X8?=
 =?us-ascii?Q?L+n7EkrOw2Pb9zMVnltVLnC3uKGgtvFcr9aXxzWt6Hv7mCYIwh2DGffOSRTj?=
 =?us-ascii?Q?ju4Si23u6oFHN6GivwcUI1F7exyAZrNs8xED3XyQ2+RWarONu7EPgPNwy6v6?=
 =?us-ascii?Q?AnDhLjmInZLas0TMfbIFegPOBl3oUNSTSR2gXTN9SSggTkmLmVHFyVW8fu00?=
 =?us-ascii?Q?RRsZxgfDEgIuLQbvoEABi7dv77mrWgD/Q5S0Hj9LKJ4iwn+P7r90DH69NdmX?=
 =?us-ascii?Q?rd32PVNnFWmDI9kiepmwo5Ref10GE3le2P5kb7QNHnOClhxG4A58IW8mbWrE?=
 =?us-ascii?Q?tsCAEV9/LARGhebF5hLiaS8KPwNgm1isAKarGiPyc8CRjmt9gyRst4cHzPtI?=
 =?us-ascii?Q?6XEOndaKsZBQDmjgDd7zya8Fwglkqr2rrPsLZP2h9Eu+gdK11hFz+jXfFygE?=
 =?us-ascii?Q?ItBcDL6UA0Ul+Zq8l9wkm84cb8s9R04WG+srrlFJbl5DexxX5kXKZdtl/7ai?=
 =?us-ascii?Q?1EejDVsoNI2GsjN3tsRM4cErjIE8uKd6r7E/3bd4vNO79MKwqWHFeyIBy05E?=
 =?us-ascii?Q?t5tNLYjUSa6kZBIDsbc6KMMvsVs6ZCjO6ISCP1NH5jHO3DS6EDNQCEanQYV6?=
 =?us-ascii?Q?/0zGC6pQsJCcTMxP9HZofZZ2ipBPm4wX/z/rrGqz4fRaOY7jBpIvliwppJk3?=
 =?us-ascii?Q?RN9kYdjnmWqsugF1vsOR4uGR7wWNs6FZMNBvksO42xOOQYzmCJUJrBjPD2Oh?=
 =?us-ascii?Q?ytltbGmi6G7vmAnPSmEunDeudaVuUa8Ca75L330ELbjrpy5hcmmmNbsFU3J/?=
 =?us-ascii?Q?R5j8ExGUd4pjFrSmRyFF9hBAjraBRz7MdE8LdBJ5I30QP7mXc6PT78TIuKWX?=
 =?us-ascii?Q?N6OqvQJO/fh+4DV9CSnAWFI6xulrOqW7K5Z03btHIKYle7zgbfw5yZfI4s5W?=
 =?us-ascii?Q?6oBlD3d2HCbaybbBUwLei3lE3eQ/+301Ldlkiuk8rKxb89s0y+HKXnsIgGh/?=
 =?us-ascii?Q?dtg3hPCKP9eNe3G+4NDvQEVaesonWgbv1QZyXxdJ?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 6077a7e9-b25c-464d-ae92-08dab713af27
X-MS-Exchange-CrossTenant-AuthSource: DU0PR04MB9417.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Oct 2022 05:33:59.8783
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Bp185UxY4beit4P++Y7+X2x5mGG6+XciyZd4QEgdBZfQQs8bV4jF/NXoIDb8zZ5iBgLriksfKeNQCT8obOWfQw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS8PR04MB8739
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,URIBL_BLOCKED autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, Oct 25, 2022 at 01:32:44PM -0500, Shenwei Wang wrote:
>On i.MX8QM/QXP/DXL SoCs, even a GPIO is selected as the wakeup source,
>the GPIO block will be powered off when system enters into suspend
>state. This can greatly reduce the power consumption of suspend state
>because the whole partition can be shutdown. This is called PAD wakeup
>feature on i.MX8x platform.
>
>This patch adds the noirq suspend/resume hooks and uses the pad wakeup
>feature as the default wakeup method for GPIO modules on
>i.MX8QM/QXP/DXL platforms.
>
>Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
>---
> drivers/gpio/gpio-mxc.c | 91 ++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 90 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
>index c871602fc5ba..a6fe5ed3d320 100644
>--- a/drivers/gpio/gpio-mxc.c
>+++ b/drivers/gpio/gpio-mxc.c
>@@ -24,6 +24,12 @@
> #include <linux/of_device.h>
> #include <linux/bug.h>
> 
>+#define IMX_SCU_WAKEUP_OFF		0
>+#define IMX_SCU_WAKEUP_LOW_LVL		4
>+#define IMX_SCU_WAKEUP_FALL_EDGE	5
>+#define IMX_SCU_WAKEUP_RISE_EDGE	6
>+#define IMX_SCU_WAKEUP_HIGH_LVL		7
>+
> /* device type dependent stuff */
> struct mxc_gpio_hwdata {
> 	unsigned dr_reg;
>@@ -61,6 +67,9 @@ struct mxc_gpio_port {
> 	u32 both_edges;
> 	struct mxc_gpio_reg_saved gpio_saved_reg;
> 	bool power_off;
>+	u32 wakeup_pads;
>+	bool is_pad_wakeup;
>+	u32 pad_type[32];
> 	const struct mxc_gpio_hwdata *hwdata;
> };
> 
>@@ -130,6 +139,9 @@ static const struct of_device_id mxc_gpio_dt_ids[] = {
> 	{ .compatible = "fsl,imx31-gpio", .data = &imx31_gpio_hwdata },
> 	{ .compatible = "fsl,imx35-gpio", .data = &imx35_gpio_hwdata },
> 	{ .compatible = "fsl,imx7d-gpio", .data = &imx35_gpio_hwdata },
>+	{ .compatible = "fsl,imx8dxl-gpio", .data = &imx35_gpio_hwdata },
>+	{ .compatible = "fsl,imx8qm-gpio", .data = &imx35_gpio_hwdata },
>+	{ .compatible = "fsl,imx8qxp-gpio", .data = &imx35_gpio_hwdata },
> 	{ /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, mxc_gpio_dt_ids);
>@@ -203,6 +215,7 @@ static int gpio_set_irq_type(struct irq_data *d, u32 type)
> 	}
> 
> 	writel(1 << gpio_idx, port->base + GPIO_ISR);
>+	port->pad_type[gpio_idx] = type;
> 
> 	return 0;
> }
>@@ -254,6 +267,9 @@ static void mx3_gpio_irq_handler(struct irq_desc *desc)
> 	struct mxc_gpio_port *port = irq_desc_get_handler_data(desc);
> 	struct irq_chip *chip = irq_desc_get_chip(desc);
> 
>+	if (port->is_pad_wakeup)
>+		return;
>+
> 	chained_irq_enter(chip, desc);
> 
> 	irq_stat = readl(port->base + GPIO_ISR) & readl(port->base + GPIO_IMR);
>@@ -306,11 +322,13 @@ static int gpio_set_wake_irq(struct irq_data *d, u32 enable)
> 			ret = enable_irq_wake(port->irq_high);
> 		else
> 			ret = enable_irq_wake(port->irq);
>+		port->wakeup_pads |= (1<<gpio_idx);

"1 << gpio_idx"

> 	} else {
> 		if (port->irq_high && (gpio_idx >= 16))
> 			ret = disable_irq_wake(port->irq_high);
> 		else
> 			ret = disable_irq_wake(port->irq);
>+		port->wakeup_pads &= ~(1<<gpio_idx);

"1 << gpio_idx"

> 	}
> 
> 	return ret;
>@@ -365,7 +383,6 @@ static int mxc_gpio_probe(struct platform_device *pdev)
> 		return -ENOMEM;
> 
> 	port->dev = &pdev->dev;
>-
> 	port->hwdata = device_get_match_data(&pdev->dev);
> 
> 	port->base = devm_platform_ioremap_resource(pdev, 0);
>@@ -498,6 +515,77 @@ static void mxc_gpio_restore_regs(struct mxc_gpio_port *port)
> 	writel(port->gpio_saved_reg.dr, port->base + GPIO_DR);
> }
> 
>+static int mxc_gpio_generic_config(struct mxc_gpio_port *port,
>+		unsigned int offset, unsigned long config)
>+{
>+	struct device_node *np = port->dev->of_node;
>+
>+	if (of_device_is_compatible(np, "fsl,imx8dxl-gpio") ||
>+		of_device_is_compatible(np, "fsl,imx8qxp-gpio") ||
>+		of_device_is_compatible(np, "fsl,imx8qm-gpio"))
>+		return gpiochip_generic_config(&port->gc, offset, config);

checkpatch should report warning.

>+
>+	return 0;
>+}
>+
>+static void mxc_gpio_set_pad_wakeup(struct mxc_gpio_port *port, bool enable)
>+{
>+	unsigned long config;
>+	int i, type;
>+
>+	static const u32 pad_type_map[] = {
>+		IMX_SCU_WAKEUP_OFF,		/* 0 */
>+		IMX_SCU_WAKEUP_RISE_EDGE,	/* IRQ_TYPE_EDGE_RISING */
>+		IMX_SCU_WAKEUP_FALL_EDGE,	/* IRQ_TYPE_EDGE_FALLING */
>+		IMX_SCU_WAKEUP_FALL_EDGE,	/* IRQ_TYPE_EDGE_BOTH */
>+		IMX_SCU_WAKEUP_HIGH_LVL,	/* IRQ_TYPE_LEVEL_HIGH */
>+		IMX_SCU_WAKEUP_OFF,		/* 5 */
>+		IMX_SCU_WAKEUP_OFF,		/* 6 */
>+		IMX_SCU_WAKEUP_OFF,		/* 7 */
>+		IMX_SCU_WAKEUP_LOW_LVL,		/* IRQ_TYPE_LEVEL_LOW */
>+	};
>+
>+	for (i = 0; i < 32; i++) {
>+		if ((port->wakeup_pads & (1<<i))) {
>+			type = port->pad_type[i];
>+			if (enable)
>+				config = pad_type_map[type];
>+			else
>+				config = IMX_SCU_WAKEUP_OFF;
>+			mxc_gpio_generic_config(port, i, config);
>+		}
>+	}
>+}
>+
>+static int __maybe_unused mxc_gpio_noirq_suspend(struct device *dev)
>+{
>+	struct platform_device *pdev = to_platform_device(dev);
>+	struct mxc_gpio_port *port = platform_get_drvdata(pdev);
>+
>+	if (port->wakeup_pads > 0) {
>+		mxc_gpio_set_pad_wakeup(port, true);
>+		port->is_pad_wakeup = true;
>+	}
>+
>+	return 0;
>+}
>+
>+static int __maybe_unused mxc_gpio_noirq_resume(struct device *dev)
>+{
>+	struct platform_device *pdev = to_platform_device(dev);
>+	struct mxc_gpio_port *port = platform_get_drvdata(pdev);
>+
>+	if (port->wakeup_pads > 0)
>+		mxc_gpio_set_pad_wakeup(port, false);
>+	port->is_pad_wakeup = false;
>+
>+	return 0;
>+}
>+
>+static const struct dev_pm_ops mxc_gpio_dev_pm_ops = {
>+	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(mxc_gpio_noirq_suspend, mxc_gpio_noirq_resume)
>+};
>+
> static int mxc_gpio_syscore_suspend(void)
> {
> 	struct mxc_gpio_port *port;
>@@ -537,6 +625,7 @@ static struct platform_driver mxc_gpio_driver = {
> 		.name	= "gpio-mxc",
> 		.of_match_table = mxc_gpio_dt_ids,
> 		.suppress_bind_attrs = true,
>+		.pm = &mxc_gpio_dev_pm_ops,
> 	},
> 	.probe		= mxc_gpio_probe,
> };


Except the format issue, patch looks good to me.

Regards,
Peng.

>-- 
>2.34.1
>

--