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@xxxxxx> 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