* [PATCH v3] PCI: imx6: Replace legacy gpio interface for gpiod interface @ 2021-11-02 1:03 Maíra Canal 2022-04-08 14:23 ` Lorenzo Pieralisi 2022-04-20 23:24 ` Linus Walleij 0 siblings, 2 replies; 10+ messages in thread From: Maíra Canal @ 2021-11-02 1:03 UTC (permalink / raw) To: hongxing.zhu, l.stach, lorenzo.pieralisi, robh, bhelgaas, helgaas, shawnguo, s.hauer Cc: kernel, linux-imx, linux-pci, linux-arm-kernel, linux-kernel Considering the current transition of the GPIO subsystem, remove all dependencies of the legacy GPIO interface (linux/gpio.h and linux /of_gpio.h) and replace it with the descriptor-based GPIO approach. Signed-off-by: Maíra Canal <maira.canal@usp.br> --- V1 -> V2: Rewrite commit log and subject line to match PCI subsystem standard V2 -> v3: Change gpiod_set_value_cansleep for gpiod_set_raw_value_cansleep --- drivers/pci/controller/dwc/pci-imx6.c | 30 +++++++++------------------ 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c index 80fc98acf097..f08865ac0b40 100644 --- a/drivers/pci/controller/dwc/pci-imx6.c +++ b/drivers/pci/controller/dwc/pci-imx6.c @@ -11,13 +11,12 @@ #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_gpio.h> #include <linux/of_device.h> #include <linux/of_address.h> #include <linux/pci.h> @@ -63,7 +62,7 @@ struct imx6_pcie_drvdata { struct imx6_pcie { struct dw_pcie *pci; - int reset_gpio; + struct gpio_desc *reset_gpio; bool gpio_active_high; struct clk *pcie_bus; struct clk *pcie_phy; @@ -526,11 +525,11 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie) usleep_range(200, 500); /* Some boards don't have PCIe reset GPIO. */ - if (gpio_is_valid(imx6_pcie->reset_gpio)) { - gpio_set_value_cansleep(imx6_pcie->reset_gpio, + if (imx6_pcie->reset_gpio) { + gpiod_set_raw_value_cansleep(imx6_pcie->reset_gpio, imx6_pcie->gpio_active_high); msleep(100); - gpio_set_value_cansleep(imx6_pcie->reset_gpio, + gpiod_set_raw_value_cansleep(imx6_pcie->reset_gpio, !imx6_pcie->gpio_active_high); } @@ -1025,22 +1024,13 @@ 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_gpio = devm_gpiod_get_optional(dev, "reset", + imx6_pcie->gpio_active_high ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW); + if (IS_ERR(imx6_pcie->reset_gpio)) + return dev_err_probe(dev, PTR_ERR(imx6_pcie->reset_gpio), + "unable to get reset gpio\n"); /* Fetch clocks */ imx6_pcie->pcie_phy = devm_clk_get(dev, "pcie_phy"); -- 2.31.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3] PCI: imx6: Replace legacy gpio interface for gpiod interface 2021-11-02 1:03 [PATCH v3] PCI: imx6: Replace legacy gpio interface for gpiod interface Maíra Canal @ 2022-04-08 14:23 ` Lorenzo Pieralisi 2022-04-08 14:46 ` Maíra Canal 2022-04-20 23:24 ` Linus Walleij 1 sibling, 1 reply; 10+ messages in thread From: Lorenzo Pieralisi @ 2022-04-08 14:23 UTC (permalink / raw) To: Maíra Canal, l.stach Cc: hongxing.zhu, robh, bhelgaas, helgaas, shawnguo, s.hauer, kernel, linux-imx, linux-pci, linux-arm-kernel, linux-kernel On Mon, Nov 01, 2021 at 10:03:11PM -0300, Maíra Canal wrote: > Considering the current transition of the GPIO subsystem, remove all > dependencies of the legacy GPIO interface (linux/gpio.h and linux > /of_gpio.h) and replace it with the descriptor-based GPIO approach. > > Signed-off-by: Maíra Canal <maira.canal@usp.br> > --- > V1 -> V2: Rewrite commit log and subject line to match PCI subsystem standard > V2 -> v3: Change gpiod_set_value_cansleep for gpiod_set_raw_value_cansleep > --- > drivers/pci/controller/dwc/pci-imx6.c | 30 +++++++++------------------ > 1 file changed, 10 insertions(+), 20 deletions(-) Maira, Lucas, what's this patch status ? Please let me know. Thanks, Lorenzo > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > index 80fc98acf097..f08865ac0b40 100644 > --- a/drivers/pci/controller/dwc/pci-imx6.c > +++ b/drivers/pci/controller/dwc/pci-imx6.c > @@ -11,13 +11,12 @@ > #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_gpio.h> > #include <linux/of_device.h> > #include <linux/of_address.h> > #include <linux/pci.h> > @@ -63,7 +62,7 @@ struct imx6_pcie_drvdata { > > struct imx6_pcie { > struct dw_pcie *pci; > - int reset_gpio; > + struct gpio_desc *reset_gpio; > bool gpio_active_high; > struct clk *pcie_bus; > struct clk *pcie_phy; > @@ -526,11 +525,11 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie) > usleep_range(200, 500); > > /* Some boards don't have PCIe reset GPIO. */ > - if (gpio_is_valid(imx6_pcie->reset_gpio)) { > - gpio_set_value_cansleep(imx6_pcie->reset_gpio, > + if (imx6_pcie->reset_gpio) { > + gpiod_set_raw_value_cansleep(imx6_pcie->reset_gpio, > imx6_pcie->gpio_active_high); > msleep(100); > - gpio_set_value_cansleep(imx6_pcie->reset_gpio, > + gpiod_set_raw_value_cansleep(imx6_pcie->reset_gpio, > !imx6_pcie->gpio_active_high); > } > > @@ -1025,22 +1024,13 @@ 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_gpio = devm_gpiod_get_optional(dev, "reset", > + imx6_pcie->gpio_active_high ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW); > + if (IS_ERR(imx6_pcie->reset_gpio)) > + return dev_err_probe(dev, PTR_ERR(imx6_pcie->reset_gpio), > + "unable to get reset gpio\n"); > > /* Fetch clocks */ > imx6_pcie->pcie_phy = devm_clk_get(dev, "pcie_phy"); > -- > 2.31.1 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] PCI: imx6: Replace legacy gpio interface for gpiod interface 2022-04-08 14:23 ` Lorenzo Pieralisi @ 2022-04-08 14:46 ` Maíra Canal 2022-04-11 11:59 ` Francesco Dolcini 2022-04-11 15:25 ` Lorenzo Pieralisi 0 siblings, 2 replies; 10+ messages in thread From: Maíra Canal @ 2022-04-08 14:46 UTC (permalink / raw) To: Lorenzo Pieralisi Cc: l.stach, hongxing.zhu, robh, bhelgaas, helgaas, shawnguo, s.hauer, kernel, linux-imx, linux-pci, linux-arm-kernel, linux-kernel On Fri, Apr 08, 2022 at 03:23:39PM +0100, Lorenzo Pieralisi wrote: > On Mon, Nov 01, 2021 at 10:03:11PM -0300, Maíra Canal wrote: > > Considering the current transition of the GPIO subsystem, remove all > > dependencies of the legacy GPIO interface (linux/gpio.h and linux > > /of_gpio.h) and replace it with the descriptor-based GPIO approach. > > > > Signed-off-by: Maíra Canal <maira.canal@usp.br> > > --- > > V1 -> V2: Rewrite commit log and subject line to match PCI subsystem standard > > V2 -> v3: Change gpiod_set_value_cansleep for gpiod_set_raw_value_cansleep > > --- > > drivers/pci/controller/dwc/pci-imx6.c | 30 +++++++++------------------ > > 1 file changed, 10 insertions(+), 20 deletions(-) > > Maira, Lucas, > > what's this patch status ? Please let me know. Lorenzo, Thank you for the feedback. Since I sent v3, I didn't get any feedback from the community. If you have any feedback, I would gladly work on it. Thanks, Maíra Canal > > Thanks, > Lorenzo > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > > index 80fc98acf097..f08865ac0b40 100644 > > --- a/drivers/pci/controller/dwc/pci-imx6.c > > +++ b/drivers/pci/controller/dwc/pci-imx6.c > > @@ -11,13 +11,12 @@ > > #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_gpio.h> > > #include <linux/of_device.h> > > #include <linux/of_address.h> > > #include <linux/pci.h> > > @@ -63,7 +62,7 @@ struct imx6_pcie_drvdata { > > > > struct imx6_pcie { > > struct dw_pcie *pci; > > - int reset_gpio; > > + struct gpio_desc *reset_gpio; > > bool gpio_active_high; > > struct clk *pcie_bus; > > struct clk *pcie_phy; > > @@ -526,11 +525,11 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie) > > usleep_range(200, 500); > > > > /* Some boards don't have PCIe reset GPIO. */ > > - if (gpio_is_valid(imx6_pcie->reset_gpio)) { > > - gpio_set_value_cansleep(imx6_pcie->reset_gpio, > > + if (imx6_pcie->reset_gpio) { > > + gpiod_set_raw_value_cansleep(imx6_pcie->reset_gpio, > > imx6_pcie->gpio_active_high); > > msleep(100); > > - gpio_set_value_cansleep(imx6_pcie->reset_gpio, > > + gpiod_set_raw_value_cansleep(imx6_pcie->reset_gpio, > > !imx6_pcie->gpio_active_high); > > } > > > > @@ -1025,22 +1024,13 @@ 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_gpio = devm_gpiod_get_optional(dev, "reset", > > + imx6_pcie->gpio_active_high ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW); > > + if (IS_ERR(imx6_pcie->reset_gpio)) > > + return dev_err_probe(dev, PTR_ERR(imx6_pcie->reset_gpio), > > + "unable to get reset gpio\n"); > > > > /* Fetch clocks */ > > imx6_pcie->pcie_phy = devm_clk_get(dev, "pcie_phy"); > > -- > > 2.31.1 > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] PCI: imx6: Replace legacy gpio interface for gpiod interface 2022-04-08 14:46 ` Maíra Canal @ 2022-04-11 11:59 ` Francesco Dolcini 2022-04-11 16:03 ` Lucas Stach 2022-04-11 15:25 ` Lorenzo Pieralisi 1 sibling, 1 reply; 10+ messages in thread From: Francesco Dolcini @ 2022-04-11 11:59 UTC (permalink / raw) To: Maíra Canal Cc: Lorenzo Pieralisi, l.stach, hongxing.zhu, robh, bhelgaas, helgaas, shawnguo, s.hauer, kernel, linux-imx, linux-pci, linux-arm-kernel, linux-kernel On Fri, Apr 08, 2022 at 11:46:32AM -0300, Maíra Canal wrote: > On Fri, Apr 08, 2022 at 03:23:39PM +0100, Lorenzo Pieralisi wrote: > > On Mon, Nov 01, 2021 at 10:03:11PM -0300, Maíra Canal wrote: > > > Considering the current transition of the GPIO subsystem, remove all > > > dependencies of the legacy GPIO interface (linux/gpio.h and linux > > > /of_gpio.h) and replace it with the descriptor-based GPIO approach. > > > > > > Signed-off-by: Maíra Canal <maira.canal@usp.br> > > > --- > > > V1 -> V2: Rewrite commit log and subject line to match PCI subsystem standard > > > V2 -> v3: Change gpiod_set_value_cansleep for gpiod_set_raw_value_cansleep > > > --- > > > drivers/pci/controller/dwc/pci-imx6.c | 30 +++++++++------------------ > > > 1 file changed, 10 insertions(+), 20 deletions(-) > > > > Maira, Lucas, > > > > what's this patch status ? Please let me know. > > Thank you for the feedback. Since I sent v3, I didn't get any feedback from the community. > > If you have any feedback, I would gladly work on it. Just for you to know that it will likely conflict with 'PCI: imx6: Fix PERST# start-up sequence' [0] that is also still waiting for some additional feedback. Francesco [0] https://lore.kernel.org/all/20220404081509.94356-1-francesco.dolcini@toradex.com/ ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] PCI: imx6: Replace legacy gpio interface for gpiod interface 2022-04-11 11:59 ` Francesco Dolcini @ 2022-04-11 16:03 ` Lucas Stach 2022-04-12 11:02 ` Maíra Canal 0 siblings, 1 reply; 10+ messages in thread From: Lucas Stach @ 2022-04-11 16:03 UTC (permalink / raw) To: Francesco Dolcini, Maíra Canal, Lorenzo Pieralisi Cc: hongxing.zhu, robh, bhelgaas, helgaas, shawnguo, s.hauer, kernel, linux-imx, linux-pci, linux-arm-kernel, linux-kernel Hi Maíra, hi Lorenzo, Am Montag, dem 11.04.2022 um 13:59 +0200 schrieb Francesco Dolcini: > On Fri, Apr 08, 2022 at 11:46:32AM -0300, Maíra Canal wrote: > > On Fri, Apr 08, 2022 at 03:23:39PM +0100, Lorenzo Pieralisi wrote: > > > On Mon, Nov 01, 2021 at 10:03:11PM -0300, Maíra Canal wrote: > > > > Considering the current transition of the GPIO subsystem, remove all > > > > dependencies of the legacy GPIO interface (linux/gpio.h and linux > > > > /of_gpio.h) and replace it with the descriptor-based GPIO approach. > > > > > > > > Signed-off-by: Maíra Canal <maira.canal@usp.br> > > > > --- > > > > V1 -> V2: Rewrite commit log and subject line to match PCI subsystem standard > > > > V2 -> v3: Change gpiod_set_value_cansleep for gpiod_set_raw_value_cansleep > > > > --- > > > > drivers/pci/controller/dwc/pci-imx6.c | 30 +++++++++------------------ > > > > 1 file changed, 10 insertions(+), 20 deletions(-) > > > > > > Maira, Lucas, > > > > > > what's this patch status ? Please let me know. The patch looks fine to me now, however... > > > > Thank you for the feedback. Since I sent v3, I didn't get any feedback from the community. > > > > If you have any feedback, I would gladly work on it. > > Just for you to know that it will likely conflict with > 'PCI: imx6: Fix PERST# start-up sequence' [0] that is also still waiting > for some additional feedback. ... they do conflict and I guess the functional fix for the reset sequence should have priority over this cleanup. Maíra, may I ask you to respin your patch as soon as Francescos fix is in? I promise to not let it fall through the cracks again. Regards, Lucas ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] PCI: imx6: Replace legacy gpio interface for gpiod interface 2022-04-11 16:03 ` Lucas Stach @ 2022-04-12 11:02 ` Maíra Canal 0 siblings, 0 replies; 10+ messages in thread From: Maíra Canal @ 2022-04-12 11:02 UTC (permalink / raw) To: Lucas Stach Cc: Francesco Dolcini, Lorenzo Pieralisi, hongxing.zhu, robh, bhelgaas, helgaas, shawnguo, s.hauer, kernel, linux-imx, linux-pci, linux-arm-kernel, linux-kernel On 04/11, Lucas Stach wrote: > Hi Maíra, hi Lorenzo, > > Am Montag, dem 11.04.2022 um 13:59 +0200 schrieb Francesco Dolcini: > > On Fri, Apr 08, 2022 at 11:46:32AM -0300, Maíra Canal wrote: > > > On Fri, Apr 08, 2022 at 03:23:39PM +0100, Lorenzo Pieralisi wrote: > > > > On Mon, Nov 01, 2021 at 10:03:11PM -0300, Maíra Canal wrote: > > > > > Considering the current transition of the GPIO subsystem, remove all > > > > > dependencies of the legacy GPIO interface (linux/gpio.h and linux > > > > > /of_gpio.h) and replace it with the descriptor-based GPIO approach. > > > > > > > > > > Signed-off-by: Maíra Canal <maira.canal@usp.br> > > > > > --- > > > > > V1 -> V2: Rewrite commit log and subject line to match PCI subsystem standard > > > > > V2 -> v3: Change gpiod_set_value_cansleep for gpiod_set_raw_value_cansleep > > > > > --- > > > > > drivers/pci/controller/dwc/pci-imx6.c | 30 +++++++++------------------ > > > > > 1 file changed, 10 insertions(+), 20 deletions(-) > > ... they do conflict and I guess the functional fix for the reset > sequence should have priority over this cleanup. Maíra, may I ask you > to respin your patch as soon as Francescos fix is in? I promise to not > let it fall through the cracks again. Hi Lucas That's fine. After Francescos patch is applied, I resend the fix. Thank you for the feedback! Sincerly, Maíra Canal > > Regards, > Lucas > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] PCI: imx6: Replace legacy gpio interface for gpiod interface 2022-04-08 14:46 ` Maíra Canal 2022-04-11 11:59 ` Francesco Dolcini @ 2022-04-11 15:25 ` Lorenzo Pieralisi 1 sibling, 0 replies; 10+ messages in thread From: Lorenzo Pieralisi @ 2022-04-11 15:25 UTC (permalink / raw) To: Maíra Canal Cc: l.stach, hongxing.zhu, robh, bhelgaas, helgaas, shawnguo, s.hauer, kernel, linux-imx, linux-pci, linux-arm-kernel, linux-kernel, linus.walleij [+Linus] On Fri, Apr 08, 2022 at 11:46:32AM -0300, Maíra Canal wrote: > On Fri, Apr 08, 2022 at 03:23:39PM +0100, Lorenzo Pieralisi wrote: > > On Mon, Nov 01, 2021 at 10:03:11PM -0300, Maíra Canal wrote: > > > Considering the current transition of the GPIO subsystem, remove all > > > dependencies of the legacy GPIO interface (linux/gpio.h and linux > > > /of_gpio.h) and replace it with the descriptor-based GPIO approach. > > > > > > Signed-off-by: Maíra Canal <maira.canal@usp.br> > > > --- > > > V1 -> V2: Rewrite commit log and subject line to match PCI subsystem standard > > > V2 -> v3: Change gpiod_set_value_cansleep for gpiod_set_raw_value_cansleep > > > --- > > > drivers/pci/controller/dwc/pci-imx6.c | 30 +++++++++------------------ > > > 1 file changed, 10 insertions(+), 20 deletions(-) > > > > Maira, Lucas, > > > > what's this patch status ? Please let me know. > > > Lorenzo, > > Thank you for the feedback. Since I sent v3, I didn't get any feedback from the community. > > If you have any feedback, I would gladly work on it. I would ask Linus to have a look please given that it is GPIO code. Original thread: https://lore.kernel.org/linux-pci/YYCOTx68LXu1Tn1i@fedora Thanks, Lorenzo ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] PCI: imx6: Replace legacy gpio interface for gpiod interface 2021-11-02 1:03 [PATCH v3] PCI: imx6: Replace legacy gpio interface for gpiod interface Maíra Canal 2022-04-08 14:23 ` Lorenzo Pieralisi @ 2022-04-20 23:24 ` Linus Walleij 2022-04-25 12:07 ` Lucas Stach 1 sibling, 1 reply; 10+ messages in thread From: Linus Walleij @ 2022-04-20 23:24 UTC (permalink / raw) To: Maíra Canal Cc: hongxing.zhu, l.stach, lorenzo.pieralisi, robh, bhelgaas, helgaas, shawnguo, s.hauer, kernel, linux-imx, linux-pci, linux-arm-kernel, linux-kernel Hi Maira and sorry for being slow on reviews. On Tue, Nov 2, 2021 at 2:04 AM Maíra Canal <maira.canal@usp.br> wrote: > - gpio_set_value_cansleep(imx6_pcie->reset_gpio, > + gpiod_set_raw_value_cansleep(imx6_pcie->reset_gpio, > !imx6_pcie->gpio_active_high); Hm I see you got advised to use the raw api. I'm not so sure about that I like v1 better. > + imx6_pcie->reset_gpio = devm_gpiod_get_optional(dev, "reset", > + imx6_pcie->gpio_active_high ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW); > + if (IS_ERR(imx6_pcie->reset_gpio)) > + return dev_err_probe(dev, PTR_ERR(imx6_pcie->reset_gpio), > + "unable to get reset gpio\n"); Where is this descriptor coming from? Device trees? Can't we just fix the DTS file(s) in that case given how wrong they are if they don't set GPIO_ACTIVE_LOW flag on this IRQ. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] PCI: imx6: Replace legacy gpio interface for gpiod interface 2022-04-20 23:24 ` Linus Walleij @ 2022-04-25 12:07 ` Lucas Stach 2022-04-25 13:40 ` Linus Walleij 0 siblings, 1 reply; 10+ messages in thread From: Lucas Stach @ 2022-04-25 12:07 UTC (permalink / raw) To: Linus Walleij, Maíra Canal Cc: hongxing.zhu, lorenzo.pieralisi, robh, bhelgaas, helgaas, shawnguo, s.hauer, kernel, linux-imx, linux-pci, linux-arm-kernel, linux-kernel Hi Linus, Am Donnerstag, dem 21.04.2022 um 01:24 +0200 schrieb Linus Walleij: > Hi Maira and sorry for being slow on reviews. > > On Tue, Nov 2, 2021 at 2:04 AM Maíra Canal <maira.canal@usp.br> wrote: > > > - gpio_set_value_cansleep(imx6_pcie->reset_gpio, > > + gpiod_set_raw_value_cansleep(imx6_pcie->reset_gpio, > > !imx6_pcie->gpio_active_high); > > Hm I see you got advised to use the raw api. I'm not so sure about > that I like v1 better. > > > + imx6_pcie->reset_gpio = devm_gpiod_get_optional(dev, "reset", > > + imx6_pcie->gpio_active_high ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW); > > + if (IS_ERR(imx6_pcie->reset_gpio)) > > + return dev_err_probe(dev, PTR_ERR(imx6_pcie->reset_gpio), > > + "unable to get reset gpio\n"); > > Where is this descriptor coming from? Device trees? Can't we just fix the > DTS file(s) in that case given how wrong they are if they don't set > GPIO_ACTIVE_LOW flag on this IRQ. The binding explicitly describes the GPIO as not polarity aware and has a separate property "reset-gpio-active-high" to avoid breaking old DTBs. I don't think it's helpful to dismiss this explicit backward compat just because the driver code looks nicer that way. Regards, Lucas ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] PCI: imx6: Replace legacy gpio interface for gpiod interface 2022-04-25 12:07 ` Lucas Stach @ 2022-04-25 13:40 ` Linus Walleij 0 siblings, 0 replies; 10+ messages in thread From: Linus Walleij @ 2022-04-25 13:40 UTC (permalink / raw) To: Lucas Stach Cc: Maíra Canal, hongxing.zhu, lorenzo.pieralisi, robh, bhelgaas, helgaas, shawnguo, s.hauer, kernel, linux-imx, linux-pci, linux-arm-kernel, linux-kernel On Mon, Apr 25, 2022 at 2:07 PM Lucas Stach <l.stach@pengutronix.de> wrote: > The binding explicitly describes the GPIO as not polarity aware and has > a separate property "reset-gpio-active-high" to avoid breaking old > DTBs. I don't think it's helpful to dismiss this explicit backward > compat just because the driver code looks nicer that way. I see. We handle such things a specific way. Look in drivers/gpio/gpiolib-of.c, especially the function of_gpio_flags_quirks(). Here we special-case all bindings which for some reason introduced something necessary custom, like in this case not using the proper polarity flag. Add code to this file in the proper place to handle and hide the old style DTBs using "reset-gpio-active-high" as active high flag and assuming active low otherwise in this file. I imagine it begins with if (IS_ENABLED(CONFIG_PCI_IMX6)) { ... } Then modify the code in drivers/pci/controller/dwc/pci-imx6.c to act as if gpiolib handles polarity inversion. Include all changes to all files in the same patch so this is changed in tandem (one technical step). Yours, Linus Walleij ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-04-25 13:41 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-11-02 1:03 [PATCH v3] PCI: imx6: Replace legacy gpio interface for gpiod interface Maíra Canal 2022-04-08 14:23 ` Lorenzo Pieralisi 2022-04-08 14:46 ` Maíra Canal 2022-04-11 11:59 ` Francesco Dolcini 2022-04-11 16:03 ` Lucas Stach 2022-04-12 11:02 ` Maíra Canal 2022-04-11 15:25 ` Lorenzo Pieralisi 2022-04-20 23:24 ` Linus Walleij 2022-04-25 12:07 ` Lucas Stach 2022-04-25 13:40 ` Linus Walleij
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).