From: Vidya Sagar <vidyas@nvidia.com>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: devicetree@vger.kernel.org, lorenzo.pieralisi@arm.com,
mmaddireddy@nvidia.com, kthota@nvidia.com,
gustavo.pimentel@synopsys.com, linux-kernel@vger.kernel.org,
robh+dt@kernel.org, kishon@ti.com, thierry.reding@gmail.com,
linux-pci@vger.kernel.org, linux-tegra@vger.kernel.org,
andrew.murray@arm.com, jonathanh@nvidia.com,
linux-arm-kernel@lists.infradead.org, sagar.tv@gmail.com
Subject: Re: [PATCH 3/6] PCI: tegra: Add support for PCIe endpoint mode in Tegra194
Date: Fri, 29 Nov 2019 18:52:47 +0530 [thread overview]
Message-ID: <7298f15d-1745-e5c9-2d08-1235a27244ae@nvidia.com> (raw)
In-Reply-To: <20191126213718.GA185422@google.com>
On 11/27/2019 3:07 AM, Bjorn Helgaas wrote:
> On Fri, Nov 22, 2019 at 04:15:02PM +0530, Vidya Sagar wrote:
>> Add support for the endpoint mode of Synopsys DesignWare core based
>> dual mode PCIe controllers present in Tegra194 SoC.
>
>> +static irqreturn_t tegra_pcie_ep_irq_handler(struct tegra_pcie_dw *pcie)
>> +{
>> + struct dw_pcie_ep *ep = &pcie->pci.ep;
>> + u32 val, tmp;
>> +
>> + val = appl_readl(pcie, APPL_INTR_STATUS_L0);
>> + if (val & APPL_INTR_STATUS_L0_LINK_STATE_INT) {
>> + val = appl_readl(pcie, APPL_INTR_STATUS_L1_0_0);
>> + appl_writel(pcie, val, APPL_INTR_STATUS_L1_0_0);
>> + if (val & APPL_INTR_STATUS_L1_0_0_HOT_RESET_DONE) {
>> + /* clear any stale PEX_RST interrupt */
>> + if (!kfifo_put(&pcie->event_fifo, EP_HOT_RST_DONE)) {
>> + dev_err(pcie->dev, "EVENT FIFO is full\n");
>> + return IRQ_HANDLED;
>> + }
>> + wake_up(&pcie->wq);
>> + }
>> + if (val & APPL_INTR_STATUS_L1_0_0_RDLH_LINK_UP_CHGED) {
>> + tmp = appl_readl(pcie, APPL_LINK_STATUS);
>> + if (tmp & APPL_LINK_STATUS_RDLH_LINK_UP) {
>> + dev_info(pcie->dev, "Link is up with Host\n");
>> + dw_pcie_ep_linkup(ep);
>> + }
>> + }
>> + } else if (val & APPL_INTR_STATUS_L0_PCI_CMD_EN_INT) {
>
> Is it really the case that only one of
> APPL_INTR_STATUS_L0_LINK_STATE_INT and
> APPL_INTR_STATUS_L0_PCI_CMD_EN_INT can be set?
Not really.
>
> If it's possible that both could be set, maybe this should be
> something like this?
>
> int spurious = 1;
>
> if (val & APPL_INTR_STATUS_L0_LINK_STATE_INT) {
> ...
> spurious = 0;
> }
> if (val & APPL_INTR_STATUS_L0_PCI_CMD_EN_INT) {
> ...
> spurious = 0;
> }
>
> if (spurious) {
> dev_warn(...)
> }
I'll take care of this in the next patch series.
>
>> + val = appl_readl(pcie, APPL_INTR_STATUS_L1_15);
>> + appl_writel(pcie, val, APPL_INTR_STATUS_L1_15);
>> + if (val & APPL_INTR_STATUS_L1_15_CFG_BME_CHGED) {
>> + if (!kfifo_put(&pcie->event_fifo, EP_BME_CHANGE)) {
>> + dev_err(pcie->dev, "EVENT FIFO is full\n");
>> + return IRQ_HANDLED;
>> + }
>> + wake_up(&pcie->wq);
>> + }
>> + } else {
>> + dev_warn(pcie->dev, "Random interrupt (STATUS = 0x%08X)\n",
>> + val);
>> + appl_writel(pcie, val, APPL_INTR_STATUS_L0);
>> + }
>> +
>> + return IRQ_HANDLED;
>> +}
>
>> +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\n");
>
> Maybe include the invalid event value in the message?
I'll take care of this in the next patch series
>
>> + break;
>> + }
>> + }
>> +
>> + return 0;
>> +}
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-11-29 13:23 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-22 10:44 [PATCH 0/6] Add support for PCIe endpoint mode in Tegra194 Vidya Sagar
2019-11-22 10:45 ` [PATCH 1/6] soc/tegra: bpmp: Update ABI header Vidya Sagar
2019-11-22 10:45 ` [PATCH 2/6] dt-bindings: PCI: tegra: Add DT support for PCIe EP nodes in Tegra194 Vidya Sagar
2019-11-22 13:19 ` Thierry Reding
2019-11-25 7:23 ` Vidya Sagar
2019-11-25 7:33 ` Thierry Reding
2019-11-25 11:52 ` Gustavo Pimentel
2019-11-29 13:26 ` Vidya Sagar
2019-12-05 9:57 ` Vidya Sagar
2019-12-04 21:43 ` Rob Herring
2019-11-22 10:45 ` [PATCH 3/6] PCI: tegra: Add support for PCIe endpoint mode " Vidya Sagar
2019-11-26 21:37 ` Bjorn Helgaas
2019-11-29 13:22 ` Vidya Sagar [this message]
2019-11-22 10:45 ` [PATCH 4/6] arm64: tegra: Add PCIe endpoint controllers nodes for Tegra194 Vidya Sagar
2019-11-22 10:45 ` [PATCH 5/6] arm64: tegra: Enable GPIO controllers nodes for P2972-0000 platform Vidya Sagar
2019-11-22 13:20 ` Thierry Reding
2019-11-25 6:55 ` Vidya Sagar
2019-11-22 10:45 ` [PATCH 6/6] arm64: tegra: Add support for PCIe endpoint mode in " Vidya Sagar
2019-11-22 13:25 ` Thierry Reding
2019-11-25 7:00 ` Vidya Sagar
2019-11-25 7:25 ` Thierry Reding
2019-11-25 7:33 ` Vidya Sagar
2019-11-25 7:37 ` Thierry Reding
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7298f15d-1745-e5c9-2d08-1235a27244ae@nvidia.com \
--to=vidyas@nvidia.com \
--cc=andrew.murray@arm.com \
--cc=devicetree@vger.kernel.org \
--cc=gustavo.pimentel@synopsys.com \
--cc=helgaas@kernel.org \
--cc=jonathanh@nvidia.com \
--cc=kishon@ti.com \
--cc=kthota@nvidia.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=linux-tegra@vger.kernel.org \
--cc=lorenzo.pieralisi@arm.com \
--cc=mmaddireddy@nvidia.com \
--cc=robh+dt@kernel.org \
--cc=sagar.tv@gmail.com \
--cc=thierry.reding@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).