> In subject, follow capitalization convention. Use "git log --oneline". > > On Mon, Nov 11, 2024 at 11:03:10PM -0800, Jenishkumar Maheshbhai Patel > wrote: > > Added pcie reset via gpio support as described in the > > designware-pcie.txt DT binding document. > > In cases link down cause still exist in device. > > The device need to be reset to reestablish the link. > > If reset-gpio pin provided in the device tree, then the linkdown > > handle resets the device before reestablishing link. > > s/pcie/PCIe/ > s/gpio/GPIO/ > > Add blank lines between paragraphs. Rewrap to fill 75 columns. > > > Signed-off-by: Jenishkumar Maheshbhai Patel > > <mailto:jpatel2@xxxxxxxxxxx> > > --- > > drivers/pci/controller/dwc/pcie-armada8k.c | 24 > > ++++++++++++++++++++-- > > 1 file changed, 22 insertions(+), 2 deletions(-) > > > > 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> > > Preserve (mostly) alpha sorted list of includes. > > > #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); > > Needs some sort of #define for this 100 ms. > > > + gpiod_set_value_cansleep(pcie->reset_gpio, > > + (pcie->flags & > OF_GPIO_ACTIVE_LOW) ? 0 : 1); > > + } > > /* > > - * Sleep needed to make sure all pcie transactions and access > > - * are flushed before resetting the mac > > + * Sleep used for two reasons. > > + * First make sure all pcie transactions and access are flushed before > resetting the mac > > + * and second to make sure pci device is ready in case we reset the > > +device > > */ > > msleep(100); > > s/pcie/PCIe/ (throughout) > s/mac/MAC/ > > Explain the 100ms. Hopefully this is something defined by PCIe base or CEM > spec. Use or add #define as needed. > I will double check if this delay is necessary or not. > > @@ -376,6 +388,7 @@ static int armada8k_pcie_probe(struct > platform_device *pdev) > > struct armada8k_pcie *pcie; > > struct device *dev = &pdev->dev; > > struct resource *base; > > + int reset_gpio; > > int ret; > > > > pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL); @@ -420,6 > > +433,13 @@ static int armada8k_pcie_probe(struct platform_device *pdev) > > goto fail_clkreg; > > } > > > > + /* 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); > > + > > pcie->sysctrl_base = syscon_regmap_lookup_by_phandle(pdev- > >dev.of_node, > > "marvell,system- > controller"); > > if (IS_ERR(pcie->sysctrl_base)) { > > -- > > 2.25.1 > >
