From b32c012e4b98f0126aa327be2d1f409963057643 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Mon, 7 Sep 2020 13:10:34 +0200 Subject: [PATCH 1/7] PCI: aardvark: Fix compilation on s390 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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' Link: https://lore.kernel.org/r/202006211118.LxtENQfl%25lkp@intel.com Link: https://lore.kernel.org/r/20200907111038.5811-2-pali@kernel.org Fixes: 5169a9851daa ("PCI: aardvark: Issue PERST via GPIO") Reported-by: kernel test robot Signed-off-by: Pali Rohár Signed-off-by: Lorenzo Pieralisi Reviewed-by: Marek Behún --- drivers/pci/controller/pci-aardvark.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c index 1559f79e63b6..1c5f2fd47c51 100644 --- a/drivers/pci/controller/pci-aardvark.c +++ b/drivers/pci/controller/pci-aardvark.c @@ -9,7 +9,7 @@ */ #include -#include +#include #include #include #include From 7862a6134456c8b4f8c39e8c94aa97e5c2f7f2b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Mon, 7 Sep 2020 13:10:35 +0200 Subject: [PATCH 2/7] PCI: aardvark: Check for errors from pci_bridge_emul_init() call MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Function pci_bridge_emul_init() may fail so correctly check for errors. Link: https://lore.kernel.org/r/20200907111038.5811-3-pali@kernel.org Fixes: 8a3ebd8de328 ("PCI: aardvark: Implement emulated root PCI bridge config space") Signed-off-by: Pali Rohár Signed-off-by: Lorenzo Pieralisi Reviewed-by: Marek Behún --- drivers/pci/controller/pci-aardvark.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c index 1c5f2fd47c51..2e2e2a2ff51d 100644 --- a/drivers/pci/controller/pci-aardvark.c +++ b/drivers/pci/controller/pci-aardvark.c @@ -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) { From d39ff8ee9f27a767b409b84d698e07ccac53dd8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Mon, 7 Sep 2020 13:10:36 +0200 Subject: [PATCH 3/7] PCI: pci-bridge-emul: Export API functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It allows kernel modules which are not compiled into kernel image to use pci-bridge-emul API functions. Link: https://lore.kernel.org/r/20200907111038.5811-4-pali@kernel.org Signed-off-by: Pali Rohár Signed-off-by: Lorenzo Pieralisi Reviewed-by: Marek Behún --- drivers/pci/pci-bridge-emul.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/pci/pci-bridge-emul.c b/drivers/pci/pci-bridge-emul.c index ccf26d12ec61..139869d50eb2 100644 --- a/drivers/pci/pci-bridge-emul.c +++ b/drivers/pci/pci-bridge-emul.c @@ -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); From 526a76991b7b791aa00037c27a653536e5f01c26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Mon, 7 Sep 2020 13:10:37 +0200 Subject: [PATCH 4/7] PCI: aardvark: Implement driver 'remove' function and allow to build it as module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Providing 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. Link: https://lore.kernel.org/r/20200907111038.5811-5-pali@kernel.org Signed-off-by: Pali Rohár Signed-off-by: Lorenzo Pieralisi Reviewed-by: Marek Behún --- drivers/pci/controller/Kconfig | 2 +- drivers/pci/controller/pci-aardvark.c | 27 ++++++++++++++++++++++++--- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig index f18c3725ef80..a7aa22512a92 100644 --- a/drivers/pci/controller/Kconfig +++ b/drivers/pci/controller/Kconfig @@ -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 diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c index 2e2e2a2ff51d..b16822e344ab 100644 --- a/drivers/pci/controller/pci-aardvark.c +++ b/drivers/pci/controller/pci-aardvark.c @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -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"); From d0c6a3475b033960e85ae2bf176b14cab0a627d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Mon, 7 Sep 2020 13:10:38 +0200 Subject: [PATCH 5/7] PCI: aardvark: Move PCIe reset card code to advk_pcie_train_link() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move 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. Link: https://lore.kernel.org/r/20200907111038.5811-6-pali@kernel.org Tested-by: Marek Behún Signed-off-by: Pali Rohár Signed-off-by: Lorenzo Pieralisi --- drivers/pci/controller/pci-aardvark.c | 64 ++++++++++++++------------- 1 file changed, 34 insertions(+), 30 deletions(-) diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c index b16822e344ab..50ab6d7519ae 100644 --- a/drivers/pci/controller/pci-aardvark.c +++ b/drivers/pci/controller/pci-aardvark.c @@ -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 @@ err: 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); /* From ea17a0f153af2cd890e4ce517130dcccaa428c13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Wed, 2 Sep 2020 16:43:43 +0200 Subject: [PATCH 6/7] phy: marvell: comphy: Convert internal SMCC firmware return codes to errno MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Driver ->power_on and ->power_off callbacks leaks internal SMCC firmware return codes to phy caller. This patch converts SMCC error codes to standard linux errno codes. Include file linux/arm-smccc.h already provides defines for SMCC error codes, so use them instead of custom driver defines. Note that return value is signed 32bit, but stored in unsigned long type with zero padding. Tested-by: Tomasz Maciej Nowak Link: https://lore.kernel.org/r/20200902144344.16684-2-pali@kernel.org Signed-off-by: Pali Rohár Signed-off-by: Lorenzo Pieralisi Reviewed-by: Rob Herring --- drivers/phy/marvell/phy-mvebu-a3700-comphy.c | 14 +++++++++++--- drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 14 +++++++++++--- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/drivers/phy/marvell/phy-mvebu-a3700-comphy.c b/drivers/phy/marvell/phy-mvebu-a3700-comphy.c index 1a138be8bd6a..810f25a47632 100644 --- a/drivers/phy/marvell/phy-mvebu-a3700-comphy.c +++ b/drivers/phy/marvell/phy-mvebu-a3700-comphy.c @@ -26,7 +26,6 @@ #define COMPHY_SIP_POWER_ON 0x82000001 #define COMPHY_SIP_POWER_OFF 0x82000002 #define COMPHY_SIP_PLL_LOCK 0x82000003 -#define COMPHY_FW_NOT_SUPPORTED (-1) #define COMPHY_FW_MODE_SATA 0x1 #define COMPHY_FW_MODE_SGMII 0x2 @@ -112,10 +111,19 @@ static int mvebu_a3700_comphy_smc(unsigned long function, unsigned long lane, unsigned long mode) { struct arm_smccc_res res; + s32 ret; arm_smccc_smc(function, lane, mode, 0, 0, 0, 0, 0, &res); + ret = res.a0; - return res.a0; + switch (ret) { + case SMCCC_RET_SUCCESS: + return 0; + case SMCCC_RET_NOT_SUPPORTED: + return -EOPNOTSUPP; + default: + return -EINVAL; + } } static int mvebu_a3700_comphy_get_fw_mode(int lane, int port, @@ -220,7 +228,7 @@ static int mvebu_a3700_comphy_power_on(struct phy *phy) } ret = mvebu_a3700_comphy_smc(COMPHY_SIP_POWER_ON, lane->id, fw_param); - if (ret == COMPHY_FW_NOT_SUPPORTED) + if (ret == -EOPNOTSUPP) dev_err(lane->dev, "unsupported SMC call, try updating your firmware\n"); diff --git a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c index e41367f36ee1..53ad127b100f 100644 --- a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c +++ b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c @@ -123,7 +123,6 @@ #define COMPHY_SIP_POWER_ON 0x82000001 #define COMPHY_SIP_POWER_OFF 0x82000002 -#define COMPHY_FW_NOT_SUPPORTED (-1) /* * A lane is described by the following bitfields: @@ -273,10 +272,19 @@ static int mvebu_comphy_smc(unsigned long function, unsigned long phys, unsigned long lane, unsigned long mode) { struct arm_smccc_res res; + s32 ret; arm_smccc_smc(function, phys, lane, mode, 0, 0, 0, 0, &res); + ret = res.a0; - return res.a0; + switch (ret) { + case SMCCC_RET_SUCCESS: + return 0; + case SMCCC_RET_NOT_SUPPORTED: + return -EOPNOTSUPP; + default: + return -EINVAL; + } } static int mvebu_comphy_get_mode(bool fw_mode, int lane, int port, @@ -819,7 +827,7 @@ static int mvebu_comphy_power_on(struct phy *phy) if (!ret) return ret; - if (ret == COMPHY_FW_NOT_SUPPORTED) + if (ret == -EOPNOTSUPP) dev_err(priv->dev, "unsupported SMC call, try updating your firmware\n"); From b0c6ae0f8948a2be6bf4e8b4bbab9ca1343289b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Wed, 2 Sep 2020 16:43:44 +0200 Subject: [PATCH 7/7] PCI: aardvark: Fix initialization with old Marvell's Arm Trusted Firmware MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Old ATF automatically power on pcie phy and does not provide SMC call for phy power on functionality which leads to aardvark initialization failure: [ 0.330134] mvebu-a3700-comphy d0018300.phy: unsupported SMC call, try updating your firmware [ 0.338846] phy phy-d0018300.phy.1: phy poweron failed --> -95 [ 0.344753] advk-pcie d0070000.pcie: Failed to initialize PHY (-95) [ 0.351160] advk-pcie: probe of d0070000.pcie failed with error -95 This patch fixes above failure by ignoring 'not supported' error in aardvark driver. In this case it is expected that phy is already power on. Tested-by: Tomasz Maciej Nowak Link: https://lore.kernel.org/r/20200902144344.16684-3-pali@kernel.org Fixes: 366697018c9a ("PCI: aardvark: Add PHY support") Signed-off-by: Pali Rohár Signed-off-by: Lorenzo Pieralisi Reviewed-by: Rob Herring Cc: # 5.8+: ea17a0f153af: phy: marvell: comphy: Convert internal SMCC firmware return codes to errno --- drivers/pci/controller/pci-aardvark.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c index 50ab6d7519ae..0be485a25327 100644 --- a/drivers/pci/controller/pci-aardvark.c +++ b/drivers/pci/controller/pci-aardvark.c @@ -1081,7 +1081,9 @@ static int advk_pcie_enable_phy(struct advk_pcie *pcie) } ret = phy_power_on(pcie->phy); - if (ret) { + if (ret == -EOPNOTSUPP) { + dev_warn(&pcie->pdev->dev, "PHY unsupported by firmware\n"); + } else if (ret) { phy_exit(pcie->phy); return ret; }