PCIe aardvark controller improvements
Subsystems (3)
Pali Rohár
maintainerv3 cover PCIe aardvark controller improvements
Hi,
we have some more improvements for PCIe aardvark controller (Armada 3720 SOC - EspressoBIN and Turris MOX).
The main improvement is that with these patches the driver can be compiled as a module, and can be reloaded at runtime.
Marek & Pali
Changes in V3:
* Rebased on top of the v5.9-rc1 release
Changes in V2 for patch 4/5:
* Protect pci_stop_root_bus() and pci_remove_root_bus() function calls by pci_lock_rescan_remove() and pci_unlock_rescan_remove()
Pali Rohár (5):
PCI: aardvark: Fix compilation on s390
PCI: aardvark: Check for errors from pci_bridge_emul_init() call
PCI: pci-bridge-emul: Export API functions
PCI: aardvark: Implement driver 'remove' function and allow to build
it as module
PCI: aardvark: Move PCIe reset card code to advk_pcie_train_link()
drivers/pci/controller/Kconfig | 2 +- drivers/pci/controller/pci-aardvark.c | 104 ++++++++++++++++---------- drivers/pci/pci-bridge-emul.c | 4 + 3 files changed, 71 insertions(+), 39 deletions(-)
Signature · 1 line
2.20.1
Pali Rohár
maintainerPATCH v3 1/5 PCI: aardvark: Fix compilation on s390
Include linux/gpio/consumer.h instead of linux/gpio.h, as is said in the latter file.
This was reported by kernel test bot when compiling for s390.
drivers/pci/controller/pci-aardvark.c:350:2: error: implicit declaration of function 'gpiod_set_value_cansleep' [-Werror,-Wimplicit-function-declaration] drivers/pci/controller/pci-aardvark.c:1074:21: error: implicit declaration of function 'devm_gpiod_get_from_of_node' [-Werror,-Wimplicit-function-declaration] drivers/pci/controller/pci-aardvark.c:1076:14: error: use of undeclared identifier 'GPIOD_OUT_LOW'
--- drivers/pci/controller/pci-aardvark.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/pci/controller/pci-aardvark.c +1 -1
@@ -9,7 +9,7 @@*/ #include <linux/delay.h> -#include <linux/gpio.h> +#include <linux/gpio/consumer.h> #include <linux/interrupt.h> #include <linux/irq.h> #include <linux/irqdomain.h>
Signature · 1 line
2.20.1
Pali Rohár
maintainerPATCH v3 2/5 PCI: aardvark: Check for errors from pci_bridge_emul_init() call
In reply to Pali RohárFunction pci_bridge_emul_init() may fail so correctly check for errors.
--- drivers/pci/controller/pci-aardvark.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
drivers/pci/controller/pci-aardvark.c +7 -4
@@ -607,7 +607,7 @@ static struct pci_bridge_emul_ops advk_pci_bridge_emul_ops = {* Initialize the configuration space of the PCI-to-PCI bridge * associated with the given PCIe interface. */ -static void advk_sw_pci_bridge_init (struct advk_pcie *pcie) +static int advk_sw_pci_bridge_init (struct advk_pcie *pcie) {struct pci_bridge_emul *bridge = &pcie->bridge;
@@ -633,8 +633,7 @@ static void advk_sw_pci_bridge_init(struct advk_pcie *pcie) bridge->data = pcie; bridge->ops = &advk_pci_bridge_emul_ops; -pci_bridge_emul_init (bridge,0 ); - +return pci_bridge_emul_init (bridge,0 ); } static bool advk_pcie_valid_device(struct advk_pcie *pcie, struct pci_bus *bus,
@@ -1167,7 +1166,11 @@ static int advk_pcie_probe(struct platform_device *pdev)advk_pcie_setup_hw (pcie); -advk_sw_pci_bridge_init (pcie); + ret =advk_sw_pci_bridge_init (pcie); +if (ret) { +dev_err (dev,"Failed to register emulated root PCI bridge\n" ); +return ret; + } ret =advk_pcie_init_irq_domain (pcie);if (ret) {
Signature · 1 line
2.20.1
Lorenzo Pieralisi
AppliedQuoted 5 lines · Pali Rohár
Hi,
we have some more improvements for PCIe aardvark controller (Armada 3720 SOC - EspressoBIN and Turris MOX).
Quoted 8 lines
The main improvement is that with these patches the driver can be compiled as a module, and can be reloaded at runtime.
Marek & Pali
Changes in V3:
* Rebased on top of the v5.9-rc1 release
Applied to pci/aardvark, thanks !
Lorenzo
Quoted 20 lines
Changes in V2 for patch 4/5:
* Protectpci_stop_root_bus()andpci_remove_root_bus()function calls bypci_lock_rescan_remove()andpci_unlock_rescan_remove()Pali Rohár (5):
PCI: aardvark: Fix compilation on s390
PCI: aardvark: Check for errors frompci_bridge_emul_init()call
PCI: pci-bridge-emul: Export API functions
PCI: aardvark: Implement driver 'remove' function and allow to build
it as module
PCI: aardvark: Move PCIe reset card code toadvk_pcie_train_link()drivers/pci/controller/Kconfig | 2 +- drivers/pci/controller/pci-aardvark.c | 104 ++++++++++++++++---------- drivers/pci/pci-bridge-emul.c | 4 + 3 files changed, 71 insertions(+), 39 deletions(-)Signature · 1 line
2.20.1
Pali Rohár
maintainerPATCH v3 3/5 PCI: pci-bridge-emul: Export API functions
In reply to Pali RohárIt allows kernel modules which are not compiled into kernel image to use pci-bridge-emul API functions.
--- drivers/pci/pci-bridge-emul.c | 4 ++++ 1 file changed, 4 insertions(+)
drivers/pci/pci-bridge-emul.c +4 -0
@@ -294,6 +294,7 @@ int pci_bridge_emul_init(struct pci_bridge_emul *bridge,return 0 ; } +EXPORT_SYMBOL_GPL (pci_bridge_emul_init);/* * Cleanup a pci_bridge_emul structure that was previously initialized
@@ -305,6 +306,7 @@ void pci_bridge_emul_cleanup(struct pci_bridge_emul *bridge)kfree (bridge->pcie_cap_regs_behavior);kfree (bridge->pci_regs_behavior); } +EXPORT_SYMBOL_GPL (pci_bridge_emul_cleanup);/* * Should be called by the PCI controller driver when reading the PCI
@@ -366,6 +368,7 @@ int pci_bridge_emul_conf_read(struct pci_bridge_emul *bridge, int where,return PCIBIOS_SUCCESSFUL ; } +EXPORT_SYMBOL_GPL (pci_bridge_emul_conf_read);/* * Should be called by the PCI controller driver when writing the PCI
@@ -430,3 +433,4 @@ int pci_bridge_emul_conf_write(struct pci_bridge_emul *bridge, int where,return PCIBIOS_SUCCESSFUL ; } +EXPORT_SYMBOL_GPL (pci_bridge_emul_conf_write);
Signature · 1 line
2.20.1
Pali Rohár
maintainerPATCH v3 4/5 PCI: aardvark: Implement driver 'remove' function and allow to build it as module
In reply to Pali RohárProviding driver's 'remove' function allows kernel to bind and unbind devices from aardvark driver. It also allows to build aardvark driver as a module.
Compiling aardvark as a module simplifies development and debugging of this driver as it can be reloaded at runtime without the need to reboot to new kernel.
--- drivers/pci/controller/Kconfig | 2 +- drivers/pci/controller/pci-aardvark.c | 27 ++++++++++++++++++++++++--- 2 files changed, 25 insertions(+), 4 deletions(-)
drivers/pci/controller/Kconfig +1 -1
@@ -12,7 +12,7 @@ config PCI_MVEBU select PCI_BRIDGE_EMUL config PCI_AARDVARK - bool "Aardvark PCIe controller" + tristate "Aardvark PCIe controller" depends on (ARCH_MVEBU && ARM64) || COMPILE_TEST depends on OF depends on PCI_MSI_IRQ_DOMAIN
drivers/pci/controller/pci-aardvark.c +24 -3
@@ -14,6 +14,7 @@#include <linux/irq.h> #include <linux/irqdomain.h> #include <linux/kernel.h> +#include <linux/module.h> #include <linux/pci.h> #include <linux/init.h> #include <linux/phy/phy.h>
@@ -1121,6 +1122,7 @@ static int advk_pcie_probe(struct platform_device *pdev) pcie =pci_host_bridge_priv (bridge); pcie->pdev = pdev; +platform_set_drvdata (pdev, pcie); pcie->base =devm_platform_ioremap_resource (pdev,0 );if (IS_ERR (pcie->base))
@@ -1198,18 +1200,37 @@ static int advk_pcie_probe(struct platform_device *pdev)return 0 ; } +static int advk_pcie_remove (struct platform_device *pdev) +{ +struct advk_pcie *pcie =platform_get_drvdata (pdev); +struct pci_host_bridge *bridge =pci_host_bridge_from_priv (pcie); + +pci_lock_rescan_remove (); +pci_stop_root_bus (bridge->bus); +pci_remove_root_bus (bridge->bus); +pci_unlock_rescan_remove (); + +advk_pcie_remove_msi_irq_domain (pcie); +advk_pcie_remove_irq_domain (pcie); + +return 0 ; +} +static const struct of_device_id advk_pcie_of_match_table[] = { { .compatible ="marvell,armada-3700-pcie" , }, {}, }; +MODULE_DEVICE_TABLE (of, advk_pcie_of_match_table);static struct platform_driver advk_pcie_driver = { .driver = { .name ="advk-pcie" , .of_match_table = advk_pcie_of_match_table, -/* Driver unloading/unbinding currently not supported */ - .suppress_bind_attrs = true, }, .probe = advk_pcie_probe, + .remove = advk_pcie_remove, }; -builtin_platform_driver (advk_pcie_driver); +module_platform_driver (advk_pcie_driver); + +MODULE_DESCRIPTION ("Aardvark PCIe controller" ); +MODULE_LICENSE ("GPL v2" );
Signature · 1 line
2.20.1
Pali Rohár
maintainerPATCH v3 5/5 PCI: aardvark: Move PCIe reset card code to advk_pcie_train_link()
In reply to Pali RohárMove code which belongs to link training (delays and resets) into advk_pcie_train_link() function, so everything related to link training, including timings is at one place.
After experiments it can be observed that link training in aardvark hardware is very sensitive to timings and delays, so it is a good idea to have this code at the same place as link training calls.
This patch does not change behavior of aardvark initialization.
--- drivers/pci/controller/pci-aardvark.c | 64 ++++++++++++++------------- 1 file changed, 34 insertions(+), 30 deletions(-)
drivers/pci/controller/pci-aardvark.c +34 -30
@@ -252,6 +252,25 @@ static void advk_pcie_wait_for_retrain(struct advk_pcie *pcie) } } +static void advk_pcie_issue_perst (struct advk_pcie *pcie) +{ +u32 reg; + +if (!pcie->reset_gpio) +return ; + +/* PERST does not work for some cards when link training is enabled */ + reg =advk_readl (pcie,PCIE_CORE_CTRL0_REG ); + reg &= ~LINK_TRAINING_EN ; +advk_writel (pcie, reg,PCIE_CORE_CTRL0_REG ); + +/* 10ms delay is needed for some cards */ +dev_info (&pcie->pdev->dev,"issuing PERST via reset GPIO for 10ms\n" ); +gpiod_set_value_cansleep (pcie->reset_gpio,1 ); +usleep_range (10000 ,11000 ); +gpiod_set_value_cansleep (pcie->reset_gpio,0 ); +} +static int advk_pcie_train_at_gen (struct advk_pcie *pcie,int gen) {int ret, neg_gen;
@@ -299,6 +318,21 @@ static void advk_pcie_train_link(struct advk_pcie *pcie)struct device *dev = &pcie->pdev->dev;int neg_gen =-1 , gen; +/* +* Reset PCIe card via PERST# signal. Some cards are not detected +* during link training when they are in some non-initial state. +*/ +advk_pcie_issue_perst (pcie); + +/* +* PERST# signal could have been asserted by pinctrl subsystem before +* probe() callback has been called or issued explicitly by reset gpio +* function advk_pcie_issue_perst(), making the endpoint going into +* fundamental reset. As required by PCI Express spec a delay for at +* least 100ms after such a reset before link training is needed. +*/ +msleep (PCI_PM_D3COLD_WAIT ); +/* * Try link training at link gen specified by device tree property * 'max-link-speed'. If this fails, iteratively train at lower gen.
@@ -331,31 +365,10 @@ static void advk_pcie_train_link(struct advk_pcie *pcie)dev_err (dev,"link never came up\n" ); } -static void advk_pcie_issue_perst (struct advk_pcie *pcie) -{ -u32 reg; - -if (!pcie->reset_gpio) -return ; - -/* PERST does not work for some cards when link training is enabled */ - reg =advk_readl (pcie,PCIE_CORE_CTRL0_REG ); - reg &= ~LINK_TRAINING_EN ; -advk_writel (pcie, reg,PCIE_CORE_CTRL0_REG ); - -/* 10ms delay is needed for some cards */ -dev_info (&pcie->pdev->dev,"issuing PERST via reset GPIO for 10ms\n" ); -gpiod_set_value_cansleep (pcie->reset_gpio,1 ); -usleep_range (10000 ,11000 ); -gpiod_set_value_cansleep (pcie->reset_gpio,0 ); -} -static void advk_pcie_setup_hw (struct advk_pcie *pcie) {u32 reg; -advk_pcie_issue_perst (pcie); -/* Enable TX */ reg =advk_readl (pcie,PCIE_CORE_REF_CLK_REG ); reg |=PCIE_CORE_REF_CLK_TX_ENABLE ;
@@ -432,15 +445,6 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie) reg |=PIO_CTRL_ADDR_WIN_DISABLE ;advk_writel (pcie, reg,PIO_CTRL ); -/* -* PERST# signal could have been asserted by pinctrl subsystem before -* probe() callback has been called or issued explicitly by reset gpio -* function advk_pcie_issue_perst(), making the endpoint going into -* fundamental reset. As required by PCI Express spec a delay for at -* least 100ms after such a reset before link training is needed. -*/ -msleep (PCI_PM_D3COLD_WAIT ); -advk_pcie_train_link (pcie);/*
Signature · 1 line
2.20.1