On 2/27/2020 5:18 PM, Lorenzo Pieralisi wrote:

External email: Use caution opening links or attachments


On Mon, Jan 13, 2020 at 11:44:11PM +0530, Vidya Sagar wrote:

quoted

Add support for the endpoint mode of Synopsys DesignWare core based
dual mode PCIe controllers present in Tegra194 SoC.

Signed-off-by: Vidya Sagar <redacted>
---
V3:
* Addressed Thierry's review comments

I need his ACK to merge this series.

[...]

quoted

+static int tegra_pcie_ep_work_thread(void *p)
+{
+     struct tegra_pcie_dw *pcie = (struct tegra_pcie_dw *)p;
+     u32 event;
+
+     while (true) {
+             wait_event_interruptible(pcie->wq,
+                                      !kfifo_is_empty(&pcie->event_fifo));
+
+             if (kthread_should_stop())
+                     break;
+
+             if (!kfifo_get(&pcie->event_fifo, &event)) {
+                     dev_warn(pcie->dev, "EVENT FIFO is empty\n");
+                     continue;
+             }
+
+             switch (event) {
+             case EP_PEX_RST_DEASSERT:
+                     dev_info(pcie->dev, "EVENT: EP_PEX_RST_DEASSERT\n");
+                     pex_ep_event_pex_rst_deassert(pcie);
+                     break;
+
+             case EP_PEX_RST_ASSERT:
+                     dev_info(pcie->dev, "EVENT: EP_PEX_RST_ASSERT\n");
+                     pex_ep_event_pex_rst_assert(pcie);
+                     break;
+
+             case EP_HOT_RST_DONE:
+                     dev_info(pcie->dev, "EVENT: EP_HOT_RST_DONE\n");
+                     pex_ep_event_hot_rst_done(pcie);
+                     break;
+
+             case EP_BME_CHANGE:
+                     dev_info(pcie->dev, "EVENT: EP_BME_CHANGE\n");
+                     pex_ep_event_bme_change(pcie);
+                     break;
+
+             case EP_EVENT_EXIT:
+                     dev_info(pcie->dev, "EVENT: EP_EVENT_EXIT\n");
+                     return 0;
+
+             default:
+                     dev_warn(pcie->dev, "Invalid PCIe EP event: %u\n",
+                              event);
+                     break;
+             }
+     }
+
+     return 0;
+}
+
+static irqreturn_t tegra_pcie_ep_pex_rst_irq(int irq, void *arg)
+{
+     struct tegra_pcie_dw *pcie = arg;
+
+     if (gpiod_get_value(pcie->pex_rst_gpiod)) {
+             if (!kfifo_put(&pcie->event_fifo, EP_PEX_RST_ASSERT)) {
+                     dev_err(pcie->dev, "EVENT FIFO is full\n");
+                     return IRQ_HANDLED;
+             }
+     } else {
+             if (!kfifo_put(&pcie->event_fifo, EP_PEX_RST_DEASSERT)) {
+                     dev_err(pcie->dev, "EVENT FIFO is full\n");
+                     return IRQ_HANDLED;
+             }
+     }
+
+     wake_up(&pcie->wq);
+
+     return IRQ_HANDLED;
+}
+

[...]

quoted

+static int tegra_pcie_config_ep(struct tegra_pcie_dw *pcie,
+                             struct platform_device *pdev)
+{
+     struct dw_pcie *pci = &pcie->pci;
+     struct device *dev = pcie->dev;
+     struct dw_pcie_ep *ep;
+     struct resource *res;
+     char *name;
+     int ret;
+
+     ep = &pci->ep;
+     ep->ops = &pcie_ep_ops;
+
+     res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space");
+     if (!res)
+             return -EINVAL;
+
+     ep->phys_base = res->start;
+     ep->addr_size = resource_size(res);
+     ep->page_size = SZ_64K;
+
+     ret = gpiod_set_debounce(pcie->pex_rst_gpiod, PERST_DEBOUNCE_TIME);
+     if (ret < 0) {
+             dev_err(dev, "Failed to set PERST GPIO debounce time: %d\n",
+                     ret);
+             return ret;
+     }
+
+     ret = gpiod_to_irq(pcie->pex_rst_gpiod);
+     if (ret < 0) {
+             dev_err(dev, "Failed to get IRQ for PERST GPIO: %d\n", ret);
+             return ret;
+     }
+     pcie->pex_rst_irq = (unsigned int)ret;
+
+     name = devm_kasprintf(dev, GFP_KERNEL, "tegra_pcie_%u_pex_rst_irq",
+                           pcie->cid);
+     if (!name) {
+             dev_err(dev, "Failed to create PERST IRQ string\n");
+             return -ENOMEM;
+     }
+
+     irq_set_status_flags(pcie->pex_rst_irq, IRQ_NOAUTOEN);
+
+     ret = devm_request_irq(dev, pcie->pex_rst_irq,
+                            tegra_pcie_ep_pex_rst_irq,
+                            IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+                            name, (void *)pcie);

I have the impression that a threaded IRQ is what you need, which
will also remove some boilerplate in the process. Any reason why
you can't use a threaded IRQ instead of a standalone kthread ?

No reason specifically. We started with kthreads and continued with 
that. I'll post a new series with threaded IRQ. It should be doable.

Thanks,
Vidya Sagar

Thanks,
Lorenzo