[PATCH] pci: pci_mvebu: Add support for reset-gpios

[Stefan Roese]

On 28.07.22 17:10, Pali Rohár wrote:
> On Thursday 28 July 2022 17:05:38 Stefan Roese wrote:
>> On 28.07.22 15:03, Pali Rohár wrote:
>>> Release PERST# signal via GPIO when "reset-gpios" is defined in device tree.
>>>
>>> Signed-off-by: Pali Rohár <pali at kernel.org>
>>> ---
>>>    drivers/pci/pci_mvebu.c | 14 ++++++++++++++
>>>    1 file changed, 14 insertions(+)
>>>
>>> diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
>>> index d80f87e0cfc6..269c027db204 100644
>>> --- a/drivers/pci/pci_mvebu.c
>>> +++ b/drivers/pci/pci_mvebu.c
>>> @@ -22,6 +22,7 @@
>>>    #include <asm/io.h>
>>>    #include <asm/arch/cpu.h>
>>>    #include <asm/arch/soc.h>
>>> +#include <asm-generic/gpio.h>
>>>    #include <linux/bitops.h>
>>>    #include <linux/delay.h>
>>>    #include <linux/errno.h>
>>> @@ -60,6 +61,7 @@ struct mvebu_pcie {
>>>    	struct resource mem;
>>>    	void __iomem *iobase;
>>>    	struct resource io;
>>> +	struct gpio_desc reset_gpio;
>>>    	u32 intregs;
>>>    	u32 port;
>>>    	u32 lane;
>>> @@ -416,6 +418,14 @@ static int mvebu_pcie_probe(struct udevice *dev)
>>>    	struct udevice *ctlr = pci_get_controller(dev);
>>>    	struct pci_controller *hose = dev_get_uclass_priv(ctlr);
>>>    	u32 reg;
>>> +	int ret;
>>> +
>>> +	/* Request for optional PERST# GPIO */
>>> +	ret = gpio_request_by_name(dev, "reset-gpios", 0, &pcie->reset_gpio, GPIOD_IS_OUT);
>>> +	if (ret && ret != -ENOENT) {
>>> +		printf("%s: unable to request reset-gpios: %d\n", pcie->name, ret);
>>> +		return ret;
>>> +	}
>>>    	/*
>>>    	 * Change Class Code of PCI Bridge device to PCI Bridge (0x600400)
>>> @@ -537,6 +547,10 @@ static int mvebu_pcie_probe(struct udevice *dev)
>>>    	pcie->cfgcache[(PCI_PREF_MEMORY_BASE - 0x10) / 4] =
>>>    		PCI_PREF_RANGE_TYPE_64 | (PCI_PREF_RANGE_TYPE_64 << 16);
>>> +	/* Release PERST# via GPIO when it was defined */
>>> +	if (dm_gpio_is_valid(&pcie->reset_gpio))
>>> +		dm_gpio_set_value(&pcie->reset_gpio, 0);
>>> +
>>
>> So you're only releasing the GPIO (setting to inactive) here. Wouldn't
>> it make sense to first use the GPIO (if available via DT) to actually
>> reset the PCI device? How is this done in the Kernel driver(s)?
>>
>> Thanks,
>> Stefan
> 
> This is something which I do not know what is the best. Kernel driver
> pci-mvebu.c has same logic - just release from reset at startup.

I see. Could you please check some other PCI Kernel drivers, how they
handle reset-gpios signalling?

Thanks,
Stefan