Received-SPF: pass (google.com: domain of linux-kernel+bounces-155828-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Tue, 23 Apr 2024 15:56:56 -0400
From: Frank Li <Frank.li@nxp.com>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
	Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= <kwilczynski@kernel.org>,
	Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= <u.kleine-koenig@pengutronix.de>,
	linux-omap@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
	imx@lists.linux.dev, linux-amlogic@lists.infradead.org,
	linux-arm-msm@vger.kernel.org, linux-tegra@vger.kernel.org,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Siddharth Vadapalli <s-vadapalli@ti.com>,
	Lorenzo Pieralisi <lpieralisi@kernel.org>,
	Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= <kw@linux.com>,
	Rob Herring <robh@kernel.org>, Bjorn Helgaas <bhelgaas@google.com>,
	Richard Zhu <hongxing.zhu@nxp.com>,
	Lucas Stach <l.stach@pengutronix.de>,
	Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>, Yue Wang <yue.wang@amlogic.com>,
	Neil Armstrong <neil.armstrong@linaro.org>,
	Kevin Hilman <khilman@baylibre.com>,
	Jerome Brunet <jbrunet@baylibre.com>,
	Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
	Xiaowei Song <songxiaowei@hisilicon.com>,
	Binghui Wang <wangbinghui@hisilicon.com>,
	Thierry Reding <thierry.reding@gmail.com>,
	Jonathan Hunter <jonathanh@nvidia.com>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Pali =?iso-8859-1?Q?Roh=E1r?= <pali@kernel.org>
Subject: Re: [PATCH v2 3/4] PCI: imx6: Convert to agnostic GPIO API
Message-ID: <ZigSiCBIwoEIPYoG@lizhi-Precision-Tower-5810>
References: <20240423172208.2723892-1-andriy.shevchenko@linux.intel.com>
 <20240423172208.2723892-4-andriy.shevchenko@linux.intel.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240423172208.2723892-4-andriy.shevchenko@linux.intel.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?MCdC1B5PQrjeleI7y0jckqqbkzlYlQkuW+5g8YsvlbxsLBALYvg2HpZXUzsK?=
 =?us-ascii?Q?8+CneFb3LR0anitBjBjKHQq7j7v9hr0Me6fjzTWyBa97X4h6tKWEDkEHr77Y?=
 =?us-ascii?Q?L9RXLVS0pvkxFxBdfmpYqd3hPhxWpI5G8sAuJOQXPdqoPmkPr6kERo6+lfs1?=
 =?us-ascii?Q?HO/es70zwEKUUcvNbUYgTMVd4YioholXcF+vqkD/uBsJlTLhBRnHcusybakh?=
 =?us-ascii?Q?jFu4tKVeTrPLP1ovgamzFdcRzrFaTuYVB7VhFaxLfyNyDFxSGALkYahzquzr?=
 =?us-ascii?Q?6fP9QSQ+YHhLw9Qq3s19lb/UQz4BQccdo0hgcYUFuQBtzuMjo+XTRtiJZ99r?=
 =?us-ascii?Q?A9ocJLWMiKetCdqnFn6JZ1jJcNiqdjvWEAqEGjJIdk2HJYZo1mI27+CsnCa2?=
 =?us-ascii?Q?/iuvQ4k0BoMVT/EmLjW2exswP9ufvX5rAK7asGqoFRc9ORC1+r6ODwDWykAS?=
 =?us-ascii?Q?v5M5ZiZopV2v85MLoYaLzp94OIMs7k0QW0ZyxKQNNKWz0EmFAAAM03W/62Zs?=
 =?us-ascii?Q?aISHGedOvkiApKyemnx2jWn0hjqWUjbCeQvCztNJsk+En9FnDp0kPAGQ6sdc?=
 =?us-ascii?Q?RqeUrAJBTq9GiRPjjFE+3JB6DzKRWqKKxH5qVpjpzS/Hw3Bq9er9IjuJQSvQ?=
 =?us-ascii?Q?YPaTqbEeUMI3Vo91Ua8GqueOy2GCjme304kiMYC/D6XmTf7C3TrAKEkYW4Ig?=
 =?us-ascii?Q?cgnQ4JMkbjB++qlEfC4hOaVKtX3ArOUQeWxY89TdZov1tqvTBdz96SwhqFyr?=
 =?us-ascii?Q?LNQrB7tXl6DjWc3nzM30rrckEjRS+nD5UhukeJl3vjgxsKg8sbvRQaUoM865?=
 =?us-ascii?Q?a1Yp2sKzI6TWNHIAGu4tSJasjlr7L1AyAgSHIj4TfNZFx54WmVHUaY04x+k3?=
 =?us-ascii?Q?FtWlCaViaStpKWXHbn4YNXz/+vlDhrwE6ky3OowzEzBB4dW4IFaGAejvv4gW?=
 =?us-ascii?Q?kEfA4NhjKv5MfZFl4pZwxw0xpTodiamMe9YQuEhCWGZNbtq0ng6OPP7ovdZD?=
 =?us-ascii?Q?P3nfoqv4NPxu+MfIZ57UQgtKv5BO9Ad1e/9O0VeXdrOp3wexs6cxYns4fAuH?=
 =?us-ascii?Q?dRkUC2u3smeXEmY+9AMy02S+XY4i58mgFyCdDJQPaBk4b4aURAKYt1NN9PgN?=
 =?us-ascii?Q?8vm7Qjj23HMJQyqie3OiwJXVwzVOxHsbRxTklcJQ98LIpCuT6UFW12ztNdQ1?=
 =?us-ascii?Q?WQP6xciq8JDAIAI3CgxONgibZYXi7uzSiXKUxzzPfB69jOPKQNnOKaLIzxs9?=
 =?us-ascii?Q?K8BAukSb6bPLo3nmypX0/X81ubKr4PiNJWAfuvfNQQFm4cwu/7iEni7JMthw?=
 =?us-ascii?Q?AkXVIoZRkajHC89pAhInycYr8ODCs/HdgCQ/867N0E60qMaeE6FSrQBwLw8E?=
 =?us-ascii?Q?XVF8hOrJMpWAWLQsM/2YWYzPAsAeIHWkALnb98FTtxTsFnPXW390Ncjc7GPz?=
 =?us-ascii?Q?J/6BjKUClgdLTZCgwqFrVuxJ6ubfkj3yjf/C6CY2BItSCeOJ21DDZgK8oqIH?=
 =?us-ascii?Q?zjmO3A4eeREN1nMxgNaaTd+dQ+WisVs6J0WS70OU2erCRAq4V0SPHpWYlKZG?=
 =?us-ascii?Q?/DjvMGcGLKipwBSX4iU=3D?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 98a2ce6e-3be1-4bf9-911a-08dc63cf8f44
X-MS-Exchange-CrossTenant-AuthSource: PAXPR04MB9642.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Apr 2024 19:57:09.3945
 (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: TZE2iZmU5m02DqKsAokM+PGDdsovzyHjsWCucO+LPJS8/os30IobokzQ5FZqDdwXmZsyNzd74JihAo1fmouk3A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS8PR04MB7557

On Tue, Apr 23, 2024 at 08:19:06PM +0300, Andy Shevchenko wrote:
> The of_gpio.h is going to be removed. In preparation of that convert
> the driver to the agnostic API.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 37 ++++++++++-----------------
>  1 file changed, 14 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 917c69edee1d..d620f1e1a43c 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -11,14 +11,13 @@
>  #include <linux/bitfield.h>
>  #include <linux/clk.h>
>  #include <linux/delay.h>
> -#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/kernel.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
>  #include <linux/mfd/syscon/imx7-iomuxc-gpr.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> -#include <linux/of_gpio.h>
>  #include <linux/of_address.h>
>  #include <linux/pci.h>
>  #include <linux/platform_device.h>
> @@ -107,7 +106,7 @@ struct imx6_pcie_drvdata {
>  
>  struct imx6_pcie {
>  	struct dw_pcie		*pci;
> -	int			reset_gpio;
> +	struct gpio_desc	*reset_gpiod;
>  	bool			gpio_active_high;
>  	bool			link_is_up;
>  	struct clk_bulk_data	clks[IMX6_PCIE_MAX_CLKS];
> @@ -721,9 +720,8 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
>  	}
>  
>  	/* Some boards don't have PCIe reset GPIO. */
> -	if (gpio_is_valid(imx6_pcie->reset_gpio))
> -		gpio_set_value_cansleep(imx6_pcie->reset_gpio,
> -					imx6_pcie->gpio_active_high);
> +	gpiod_set_raw_value_cansleep(imx6_pcie->reset_gpiod,
> +				     imx6_pcie->gpio_active_high);
>  }
>  
>  static int imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
> @@ -771,10 +769,10 @@ static int imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
>  	}
>  
>  	/* Some boards don't have PCIe reset GPIO. */
> -	if (gpio_is_valid(imx6_pcie->reset_gpio)) {
> +	if (imx6_pcie->reset_gpiod) {
>  		msleep(100);
> -		gpio_set_value_cansleep(imx6_pcie->reset_gpio,
> -					!imx6_pcie->gpio_active_high);
> +		gpiod_set_raw_value_cansleep(imx6_pcie->reset_gpiod,
> +					     !imx6_pcie->gpio_active_high);
>  		/* Wait for 100ms after PERST# deassertion (PCIe r5.0, 6.6.1) */
>  		msleep(100);
>  	}
> @@ -1285,22 +1283,15 @@ static int imx6_pcie_probe(struct platform_device *pdev)
>  		return PTR_ERR(pci->dbi_base);
>  
>  	/* Fetch GPIOs */
> -	imx6_pcie->reset_gpio = of_get_named_gpio(node, "reset-gpio", 0);
>  	imx6_pcie->gpio_active_high = of_property_read_bool(node,
>  						"reset-gpio-active-high");
> -	if (gpio_is_valid(imx6_pcie->reset_gpio)) {
> -		ret = devm_gpio_request_one(dev, imx6_pcie->reset_gpio,
> -				imx6_pcie->gpio_active_high ?
> -					GPIOF_OUT_INIT_HIGH :
> -					GPIOF_OUT_INIT_LOW,
> -				"PCIe reset");
> -		if (ret) {
> -			dev_err(dev, "unable to get reset gpio\n");
> -			return ret;
> -		}
> -	} else if (imx6_pcie->reset_gpio == -EPROBE_DEFER) {
> -		return imx6_pcie->reset_gpio;
> -	}
> +	imx6_pcie->reset_gpiod =
> +		devm_gpiod_get_optional(dev, "reset",
> +			imx6_pcie->gpio_active_high ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW);
> +	if (IS_ERR(imx6_pcie->reset_gpiod))
> +		return dev_err_probe(dev, PTR_ERR(imx6_pcie->reset_gpiod),
> +				     "unable to get reset gpio\n");

Small problem here. err message "unable to get reset gpio\n" will print
when -EPROBE_DEFER happen. EPROBE_DEFER is quite common when use i2c
expand gpio chip.

Frank

> +	gpiod_set_consumer_name(imx6_pcie->reset_gpiod, "PCIe reset");
>  
>  	if (imx6_pcie->drvdata->clks_cnt >= IMX6_PCIE_MAX_CLKS)
>  		return dev_err_probe(dev, -ENOMEM, "clks_cnt is too big\n");
> -- 
> 2.43.0.rc1.1336.g36b5255a03ac
>