On Mon, Nov 11, 2024 at 11:03:10PM -0800, Jenishkumar Maheshbhai Patel wrote:
> diff --git a/drivers/pci/controller/dwc/pcie-armada8k.c b/drivers/pci/controller/dwc/pcie-armada8k.c
> index b1b48c2016f7..9a48ef60be51 100644
> --- a/drivers/pci/controller/dwc/pcie-armada8k.c
> +++ b/drivers/pci/controller/dwc/pcie-armada8k.c
> @@ -23,6 +23,7 @@
> #include <linux/of_pci.h>
> #include <linux/mfd/syscon.h>
> #include <linux/regmap.h>
> +#include <linux/of_gpio.h>
>
> #include "pcie-designware.h"
>
> @@ -37,6 +38,8 @@ struct armada8k_pcie {
> struct regmap *sysctrl_base;
> u32 mac_rest_bitmask;
> struct work_struct recover_link_work;
> + enum of_gpio_flags flags;
> + struct gpio_desc *reset_gpio;
> };
>
> #define PCIE_VENDOR_REGS_OFFSET 0x8000
> @@ -238,9 +241,18 @@ static void armada8k_pcie_recover_link(struct work_struct *ws)
> }
> pci_lock_rescan_remove();
> pci_stop_and_remove_bus_device(root_port);
> + /* Reset device if reset gpio is set */
> + if (pcie->reset_gpio) {
> + /* assert and then deassert the reset signal */
> + gpiod_set_value_cansleep(pcie->reset_gpio, 0);
> + msleep(100);
> + gpiod_set_value_cansleep(pcie->reset_gpio,
> + (pcie->flags & OF_GPIO_ACTIVE_LOW) ? 0 : 1);
This looks wrong. resets are normally active-low.
gpiod_set_value_cansleep() should be called with '1' to indicate active
state, and '0' to indicate de-active state. DT should specify whether
the signal is active high or active low.
> + /* Config reset gpio for pcie if the reset connected to gpio */
> + reset_gpio = of_get_named_gpio_flags(pdev->dev.of_node,
> + "reset-gpios", 0,
> + &pcie->flags);
> + if (gpio_is_valid(reset_gpio))
> + pcie->reset_gpio = gpio_to_desc(reset_gpio);
> +
Just use devm_fwnode_gpiod_get() here, which will also handle the
cleanup.
To see how this should be done, look at
drivers/pci/controller/pci-mvebu.c which gets the polarity settings
correct too, as mentioned above.
Lastly, as this patch is related to the breaking DT change, it
_should_ have been sent as a patch series (which means the same
Cc list on each patch) so that reviewers can see the full story
and comments on the other patches.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
