| Message ID | 20240329090945.1097609-20-dlemoal@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Improve PCI memory mapping API | expand |
Hi Damien, kernel test robot noticed the following build errors: [auto build test ERROR on pci/next] [also build test ERROR on pci/for-linus linus/master v6.9-rc1 next-20240328] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Damien-Le-Moal/PCI-endpoint-Introduce-pci_epc_check_func/20240329-171158 base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next patch link: https://lore.kernel.org/r/20240329090945.1097609-20-dlemoal%40kernel.org patch subject: [PATCH 19/19] PCI: rockchip-ep: Handle PERST signal in endpoint mode config: i386-buildonly-randconfig-005-20240330 (https://download.01.org/0day-ci/archive/20240330/202403301809.VzsJsDFL-lkp@intel.com/config) compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240330/202403301809.VzsJsDFL-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202403301809.VzsJsDFL-lkp@intel.com/ All errors (new ones prefixed by >>): >> drivers/pci/controller/pcie-rockchip-ep.c:633:2: error: call to undeclared function 'irq_set_irq_type'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] 633 | irq_set_irq_type(ep->perst_irq, | ^ drivers/pci/controller/pcie-rockchip-ep.c:633:2: note: did you mean 'irq_set_irq_wake'? include/linux/interrupt.h:482:12: note: 'irq_set_irq_wake' declared here 482 | extern int irq_set_irq_wake(unsigned int irq, unsigned int on); | ^ >> drivers/pci/controller/pcie-rockchip-ep.c:657:2: error: call to undeclared function 'irq_set_status_flags'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] 657 | irq_set_status_flags(ep->perst_irq, IRQ_NOAUTOEN); | ^ >> drivers/pci/controller/pcie-rockchip-ep.c:657:38: error: use of undeclared identifier 'IRQ_NOAUTOEN' 657 | irq_set_status_flags(ep->perst_irq, IRQ_NOAUTOEN); | ^ 3 errors generated. vim +/irq_set_irq_type +633 drivers/pci/controller/pcie-rockchip-ep.c 620 621 static irqreturn_t rockchip_pcie_ep_perst_irq_thread(int irq, void *data) 622 { 623 struct pci_epc *epc = data; 624 struct rockchip_pcie_ep *ep = epc_get_drvdata(epc); 625 struct rockchip_pcie *rockchip = &ep->rockchip; 626 u32 perst = gpiod_get_value(rockchip->ep_gpio); 627 628 if (perst) 629 rockchip_pcie_ep_perst_assert(ep); 630 else 631 rockchip_pcie_ep_perst_deassert(ep); 632 > 633 irq_set_irq_type(ep->perst_irq, 634 (perst ? IRQF_TRIGGER_HIGH : IRQF_TRIGGER_LOW)); 635 636 return IRQ_HANDLED; 637 } 638 639 static int rockchip_pcie_ep_setup_irq(struct pci_epc *epc) 640 { 641 struct rockchip_pcie_ep *ep = epc_get_drvdata(epc); 642 struct rockchip_pcie *rockchip = &ep->rockchip; 643 struct device *dev = rockchip->dev; 644 int ret; 645 646 if (!rockchip->ep_gpio) 647 return 0; 648 649 /* PCIe reset interrupt */ 650 ep->perst_irq = gpiod_to_irq(rockchip->ep_gpio); 651 if (ep->perst_irq < 0) { 652 dev_err(dev, "No corresponding IRQ for PERST GPIO\n"); 653 return ep->perst_irq; 654 } 655 656 ep->perst_asserted = true; > 657 irq_set_status_flags(ep->perst_irq, IRQ_NOAUTOEN); 658 ret = devm_request_threaded_irq(dev, ep->perst_irq, NULL, 659 rockchip_pcie_ep_perst_irq_thread, 660 IRQF_TRIGGER_HIGH | IRQF_ONESHOT, 661 "pcie-ep-perst", epc); 662 if (ret) { 663 dev_err(dev, "Request PERST GPIO IRQ failed %d\n", ret); 664 return ret; 665 } 666 667 return 0; 668 } 669
Hi Damien, kernel test robot noticed the following build errors: [auto build test ERROR on pci/next] [also build test ERROR on pci/for-linus linus/master v6.9-rc1 next-20240328] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Damien-Le-Moal/PCI-endpoint-Introduce-pci_epc_check_func/20240329-171158 base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next patch link: https://lore.kernel.org/r/20240329090945.1097609-20-dlemoal%40kernel.org patch subject: [PATCH 19/19] PCI: rockchip-ep: Handle PERST signal in endpoint mode config: sparc-allyesconfig (https://download.01.org/0day-ci/archive/20240331/202403311327.3pGGy6Qm-lkp@intel.com/config) compiler: sparc64-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240331/202403311327.3pGGy6Qm-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202403311327.3pGGy6Qm-lkp@intel.com/ All errors (new ones prefixed by >>): drivers/pci/controller/pcie-rockchip-ep.c: In function 'rockchip_pcie_ep_perst_irq_thread': >> drivers/pci/controller/pcie-rockchip-ep.c:633:9: error: implicit declaration of function 'irq_set_irq_type'; did you mean 'irq_set_irq_wake'? [-Werror=implicit-function-declaration] 633 | irq_set_irq_type(ep->perst_irq, | ^~~~~~~~~~~~~~~~ | irq_set_irq_wake drivers/pci/controller/pcie-rockchip-ep.c: In function 'rockchip_pcie_ep_setup_irq': >> drivers/pci/controller/pcie-rockchip-ep.c:657:9: error: implicit declaration of function 'irq_set_status_flags' [-Werror=implicit-function-declaration] 657 | irq_set_status_flags(ep->perst_irq, IRQ_NOAUTOEN); | ^~~~~~~~~~~~~~~~~~~~ >> drivers/pci/controller/pcie-rockchip-ep.c:657:45: error: 'IRQ_NOAUTOEN' undeclared (first use in this function); did you mean 'IRQF_NO_AUTOEN'? 657 | irq_set_status_flags(ep->perst_irq, IRQ_NOAUTOEN); | ^~~~~~~~~~~~ | IRQF_NO_AUTOEN drivers/pci/controller/pcie-rockchip-ep.c:657:45: note: each undeclared identifier is reported only once for each function it appears in cc1: some warnings being treated as errors vim +633 drivers/pci/controller/pcie-rockchip-ep.c 620 621 static irqreturn_t rockchip_pcie_ep_perst_irq_thread(int irq, void *data) 622 { 623 struct pci_epc *epc = data; 624 struct rockchip_pcie_ep *ep = epc_get_drvdata(epc); 625 struct rockchip_pcie *rockchip = &ep->rockchip; 626 u32 perst = gpiod_get_value(rockchip->ep_gpio); 627 628 if (perst) 629 rockchip_pcie_ep_perst_assert(ep); 630 else 631 rockchip_pcie_ep_perst_deassert(ep); 632 > 633 irq_set_irq_type(ep->perst_irq, 634 (perst ? IRQF_TRIGGER_HIGH : IRQF_TRIGGER_LOW)); 635 636 return IRQ_HANDLED; 637 } 638 639 static int rockchip_pcie_ep_setup_irq(struct pci_epc *epc) 640 { 641 struct rockchip_pcie_ep *ep = epc_get_drvdata(epc); 642 struct rockchip_pcie *rockchip = &ep->rockchip; 643 struct device *dev = rockchip->dev; 644 int ret; 645 646 if (!rockchip->ep_gpio) 647 return 0; 648 649 /* PCIe reset interrupt */ 650 ep->perst_irq = gpiod_to_irq(rockchip->ep_gpio); 651 if (ep->perst_irq < 0) { 652 dev_err(dev, "No corresponding IRQ for PERST GPIO\n"); 653 return ep->perst_irq; 654 } 655 656 ep->perst_asserted = true; > 657 irq_set_status_flags(ep->perst_irq, IRQ_NOAUTOEN); 658 ret = devm_request_threaded_irq(dev, ep->perst_irq, NULL, 659 rockchip_pcie_ep_perst_irq_thread, 660 IRQF_TRIGGER_HIGH | IRQF_ONESHOT, 661 "pcie-ep-perst", epc); 662 if (ret) { 663 dev_err(dev, "Request PERST GPIO IRQ failed %d\n", ret); 664 return ret; 665 } 666 667 return 0; 668 } 669
diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c index d8e56b4a5578..501d25f7284c 100644 --- a/drivers/pci/controller/pcie-rockchip-ep.c +++ b/drivers/pci/controller/pcie-rockchip-ep.c @@ -18,6 +18,7 @@ #include <linux/sizes.h> #include <linux/workqueue.h> #include <linux/iopoll.h> +#include <linux/gpio/consumer.h> #include "pcie-rockchip.h" @@ -50,6 +51,9 @@ struct rockchip_pcie_ep { u64 irq_pci_addr; u8 irq_pci_fn; u8 irq_pending; + int perst_irq; + bool perst_asserted; + bool link_up; struct delayed_work link_training; }; @@ -464,13 +468,17 @@ static int rockchip_pcie_ep_start(struct pci_epc *epc) rockchip_pcie_write(rockchip, cfg, PCIE_CORE_PHY_FUNC_CFG); + if (rockchip->ep_gpio) + enable_irq(ep->perst_irq); + /* Enable configuration and start link training */ rockchip_pcie_write(rockchip, PCIE_CLIENT_LINK_TRAIN_ENABLE | PCIE_CLIENT_CONF_ENABLE, PCIE_CLIENT_CONFIG); - schedule_delayed_work(&ep->link_training, 0); + if (!rockchip->ep_gpio) + schedule_delayed_work(&ep->link_training, 0); return 0; } @@ -480,6 +488,11 @@ static void rockchip_pcie_ep_stop(struct pci_epc *epc) struct rockchip_pcie_ep *ep = epc_get_drvdata(epc); struct rockchip_pcie *rockchip = &ep->rockchip; + if (rockchip->ep_gpio) { + ep->perst_asserted = true; + disable_irq(ep->perst_irq); + } + cancel_delayed_work_sync(&ep->link_training); /* Stop link training and disable configuration */ @@ -542,6 +555,13 @@ static void rockchip_pcie_ep_link_training(struct work_struct *work) if (!rockchip_pcie_ep_link_up(rockchip)) goto again; + /* + * If PERST was asserted while polling the link, do not notify + * the function. + */ + if (ep->perst_asserted) + return; + val = rockchip_pcie_read(rockchip, PCIE_CLIENT_BASIC_STATUS0); dev_info(dev, "Link UP (Negociated speed: %sGT/s, width: x%lu)\n", @@ -551,6 +571,7 @@ static void rockchip_pcie_ep_link_training(struct work_struct *work) /* Notify the function */ pci_epc_linkup(ep->epc); + ep->link_up = true; return; @@ -558,6 +579,94 @@ static void rockchip_pcie_ep_link_training(struct work_struct *work) schedule_delayed_work(&ep->link_training, msecs_to_jiffies(5)); } +static void rockchip_pcie_ep_perst_assert(struct rockchip_pcie_ep *ep) +{ + struct rockchip_pcie *rockchip = &ep->rockchip; + struct device *dev = rockchip->dev; + + dev_dbg(dev, "PERST asserted, link down\n"); + + if (ep->perst_asserted) + return; + + ep->perst_asserted = true; + + cancel_delayed_work_sync(&ep->link_training); + + if (ep->link_up) { + pci_epc_linkdown(ep->epc); + ep->link_up = false; + } +} + +static void rockchip_pcie_ep_perst_deassert(struct rockchip_pcie_ep *ep) +{ + struct rockchip_pcie *rockchip = &ep->rockchip; + struct device *dev = rockchip->dev; + + dev_dbg(dev, "PERST de-asserted, starting link training\n"); + + if (!ep->perst_asserted) + return; + + ep->perst_asserted = false; + + /* Enable link re-training */ + rockchip_pcie_ep_retrain_link(rockchip); + + /* Start link training */ + schedule_delayed_work(&ep->link_training, 0); +} + +static irqreturn_t rockchip_pcie_ep_perst_irq_thread(int irq, void *data) +{ + struct pci_epc *epc = data; + struct rockchip_pcie_ep *ep = epc_get_drvdata(epc); + struct rockchip_pcie *rockchip = &ep->rockchip; + u32 perst = gpiod_get_value(rockchip->ep_gpio); + + if (perst) + rockchip_pcie_ep_perst_assert(ep); + else + rockchip_pcie_ep_perst_deassert(ep); + + irq_set_irq_type(ep->perst_irq, + (perst ? IRQF_TRIGGER_HIGH : IRQF_TRIGGER_LOW)); + + return IRQ_HANDLED; +} + +static int rockchip_pcie_ep_setup_irq(struct pci_epc *epc) +{ + struct rockchip_pcie_ep *ep = epc_get_drvdata(epc); + struct rockchip_pcie *rockchip = &ep->rockchip; + struct device *dev = rockchip->dev; + int ret; + + if (!rockchip->ep_gpio) + return 0; + + /* PCIe reset interrupt */ + ep->perst_irq = gpiod_to_irq(rockchip->ep_gpio); + if (ep->perst_irq < 0) { + dev_err(dev, "No corresponding IRQ for PERST GPIO\n"); + return ep->perst_irq; + } + + ep->perst_asserted = true; + irq_set_status_flags(ep->perst_irq, IRQ_NOAUTOEN); + ret = devm_request_threaded_irq(dev, ep->perst_irq, NULL, + rockchip_pcie_ep_perst_irq_thread, + IRQF_TRIGGER_HIGH | IRQF_ONESHOT, + "pcie-ep-perst", epc); + if (ret) { + dev_err(dev, "Request PERST GPIO IRQ failed %d\n", ret); + return ret; + } + + return 0; +} + static const struct pci_epc_features rockchip_pcie_epc_features = { .linkup_notifier = true, .msi_capable = true, @@ -721,6 +830,7 @@ static int rockchip_pcie_ep_probe(struct platform_device *pdev) rockchip->is_rc = false; rockchip->dev = dev; INIT_DELAYED_WORK(&ep->link_training, rockchip_pcie_ep_link_training); + ep->link_up = false; epc = devm_pci_epc_create(dev, &rockchip_pcie_epc_ops); if (IS_ERR(epc)) { @@ -748,7 +858,13 @@ static int rockchip_pcie_ep_probe(struct platform_device *pdev) /* Only enable function 0 by default */ rockchip_pcie_write(rockchip, BIT(0), PCIE_CORE_PHY_FUNC_CFG); + err = rockchip_pcie_ep_setup_irq(epc); + if (err < 0) + goto err_uninit_port; + return 0; +err_uninit_port: + rockchip_pcie_deinit_phys(rockchip); err_release_resources: rockchip_pcie_ep_release_resources(ep); err_disable_clocks: diff --git a/drivers/pci/controller/pcie-rockchip.c b/drivers/pci/controller/pcie-rockchip.c index dbec700ba9f9..3938c0b6b5a9 100644 --- a/drivers/pci/controller/pcie-rockchip.c +++ b/drivers/pci/controller/pcie-rockchip.c @@ -119,13 +119,11 @@ int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip) return PTR_ERR(rockchip->aclk_rst); } - if (rockchip->is_rc) { - rockchip->ep_gpio = devm_gpiod_get_optional(dev, "ep", - GPIOD_OUT_HIGH); - if (IS_ERR(rockchip->ep_gpio)) - return dev_err_probe(dev, PTR_ERR(rockchip->ep_gpio), - "failed to get ep GPIO\n"); - } + rockchip->ep_gpio = devm_gpiod_get_optional(dev, "ep", + rockchip->is_rc ? GPIOD_OUT_HIGH : GPIOD_IN); + if (IS_ERR(rockchip->ep_gpio)) + return dev_err_probe(dev, PTR_ERR(rockchip->ep_gpio), + "failed to get ep GPIO\n"); rockchip->aclk_pcie = devm_clk_get(dev, "aclk"); if (IS_ERR(rockchip->aclk_pcie)) {
Currently, the Rockchip PCIe endpoint controller driver does not handle PERST signal, which prevents detecting when link training should actually be started or if the host reset the device. This however can be supported using the controller ep_gpio, set as an input GPIO for endpoint mode. Modify the endpoint rockchip driver to get the ep_gpio and its associated interrupt which is serviced using a threaded IRQ with the function rockchip_pcie_ep_perst_irq_thread() as handler. This handler function notifies a link down event corresponding to the RC side asserting the PERST signal using pci_epc_linkdown() when the gpio is high. Once the gpio value goes down, corresponding to the RC de-asserting the PERST signal, link training is started. The polarity of the gpio interrupt trigger is changed from high to low after the RC asserted PERST, and conversely changed from low to high after the RC de-asserts PERST. Signed-off-by: Damien Le Moal <dlemoal@kernel.org> --- drivers/pci/controller/pcie-rockchip-ep.c | 118 +++++++++++++++++++++- drivers/pci/controller/pcie-rockchip.c | 12 +-- 2 files changed, 122 insertions(+), 8 deletions(-)