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 <[email protected]>
---
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");
+ 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
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 <[email protected]>
> ---
> 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
>
On Tue, Apr 23, 2024 at 03:56:56PM -0400, Frank Li wrote:
> On Tue, Apr 23, 2024 at 08:19:06PM +0300, Andy Shevchenko wrote:
..
> > + 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.
I'm not sure how you come to this conclusion. Can you elaborate, please?
P.S> I do not see a problem as described.
--
With Best Regards,
Andy Shevchenko
On Tue, Apr 23, 2024 at 11:03:59PM +0300, Andy Shevchenko wrote:
> On Tue, Apr 23, 2024 at 03:56:56PM -0400, Frank Li wrote:
> > On Tue, Apr 23, 2024 at 08:19:06PM +0300, Andy Shevchenko wrote:
>
> ...
>
> > > + 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.
>
> I'm not sure how you come to this conclusion. Can you elaborate, please?
> P.S> I do not see a problem as described.
If i2c gpio-expander driver have not load when imx6_pcie probe, I supposed
devm_gpiod_get_optional() will return -EPROBE_DEFER,
if (IS_ERR(imx6_pcie->reset_gpiod)) should be true. then dev_err_probe()
will run and print "unable to get reset gpio\n" with error code
-EPROBE_DEFER.
driver framework will retry imx6_pcie probe again when a new device appear.
it may retry sevial times utill i2c gpio-expander driver probe success or
timeout.
Frank
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
On Tue, Apr 23, 2024 at 04:16:36PM -0400, Frank Li wrote:
> On Tue, Apr 23, 2024 at 11:03:59PM +0300, Andy Shevchenko wrote:
> > On Tue, Apr 23, 2024 at 03:56:56PM -0400, Frank Li wrote:
> > > On Tue, Apr 23, 2024 at 08:19:06PM +0300, Andy Shevchenko wrote:
> >
> > ...
> >
> > > > + 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.
> >
> > I'm not sure how you come to this conclusion. Can you elaborate, please?
> > P.S> I do not see a problem as described.
>
> If i2c gpio-expander driver have not load when imx6_pcie probe, I supposed
> devm_gpiod_get_optional() will return -EPROBE_DEFER,
>
> if (IS_ERR(imx6_pcie->reset_gpiod)) should be true. then dev_err_probe()
> will run and print "unable to get reset gpio\n" with error code
> -EPROBE_DEFER.
Sorry for that. dev_err_probe() already consider this. Please forget my
comments.
dev_err_probe()
{
if (err != -EPROBE_DEFER) {
dev_err(dev, "error %pe: %pV", ERR_PTR(err), &vaf);
}
}
So:
Reviewed-by: Frank Li <[email protected]>
>
> driver framework will retry imx6_pcie probe again when a new device appear.
> it may retry sevial times utill i2c gpio-expander driver probe success or
> timeout.
>
> Frank
>
> >
> > --
> > With Best Regards,
> > Andy Shevchenko
> >
> >
On Tue, Apr 23, 2024 at 5:16 PM Frank Li <[email protected]> wrote:
> if (IS_ERR(imx6_pcie->reset_gpiod)) should be true. then dev_err_probe()
> will run and print "unable to get reset gpio\n" with error code
> -EPROBE_DEFER.
dev_err_probe() will not print an error message when the error code is
-EPROBE_DEFER.
That's exactly the point of using dev_err_probe().
On Tue, Apr 23, 2024 at 04:23:06PM -0400, Frank Li wrote:
> On Tue, Apr 23, 2024 at 04:16:36PM -0400, Frank Li wrote:
..
> Reviewed-by: Frank Li <[email protected]>
Thank you!
--
With Best Regards,
Andy Shevchenko
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 <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
- Mani
> ---
> 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");
> + 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
>
>
--
மணிவண்ணன் சதாசிவம்