ntb.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Rob Herring <robh@kernel.org>
To: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
	Jonathan Corbet <corbet@lwn.net>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Arnd Bergmann <arnd@arndb.de>, Jon Mason <jdmason@kudzu.us>,
	Dave Jiang <dave.jiang@intel.com>,
	Allen Hubbe <allenbh@gmail.com>, Tom Joseph <tjoseph@cadence.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	PCI <linux-pci@vger.kernel.org>,
	Linux Doc Mailing List <linux-doc@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	linux-ntb@googlegroups.com
Subject: Re: [PATCH v8 11/18] PCI: cadence: Implement ->msi_map_irq() ops
Date: Mon, 4 Jan 2021 08:15:00 -0700	[thread overview]
Message-ID: <CAL_JsqKT8WUVy4qhQkRuYLuqkQa11=7JzXcVxvNRsB0KFj+qVQ@mail.gmail.com> (raw)
In-Reply-To: <992b5423-89a2-a03b-539d-a9b2822f598a@ti.com>

On Mon, Jan 4, 2021 at 6:13 AM Kishon Vijay Abraham I <kishon@ti.com> wrote:
>
> Hi Rob,
>
> On 15/12/20 9:31 pm, Rob Herring wrote:
> > On Wed, Nov 11, 2020 at 9:37 AM Kishon Vijay Abraham I <kishon@ti.com> wrote:
> >>
> >> Implement ->msi_map_irq() ops in order to map physical address to
> >> MSI address and return MSI data.
> >>
> >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> >> ---
> >>  .../pci/controller/cadence/pcie-cadence-ep.c  | 53 +++++++++++++++++++
> >>  1 file changed, 53 insertions(+)
> >>
> >> diff --git a/drivers/pci/controller/cadence/pcie-cadence-ep.c b/drivers/pci/controller/cadence/pcie-cadence-ep.c
> >> index 84cc58dc8512..1fe6b8baca97 100644
> >> --- a/drivers/pci/controller/cadence/pcie-cadence-ep.c
> >> +++ b/drivers/pci/controller/cadence/pcie-cadence-ep.c
> >> @@ -382,6 +382,57 @@ static int cdns_pcie_ep_send_msi_irq(struct cdns_pcie_ep *ep, u8 fn,
> >>         return 0;
> >>  }
> >>
> >> +static int cdns_pcie_ep_map_msi_irq(struct pci_epc *epc, u8 fn,
> >> +                                   phys_addr_t addr, u8 interrupt_num,
> >> +                                   u32 entry_size, u32 *msi_data,
> >> +                                   u32 *msi_addr_offset)
> >> +{
> >> +       struct cdns_pcie_ep *ep = epc_get_drvdata(epc);
> >> +       u32 cap = CDNS_PCIE_EP_FUNC_MSI_CAP_OFFSET;
> >> +       struct cdns_pcie *pcie = &ep->pcie;
> >> +       u64 pci_addr, pci_addr_mask = 0xff;
> >> +       u16 flags, mme, data, data_mask;
> >> +       u8 msi_count;
> >> +       int ret;
> >> +       int i;
> >> +
> >
> >
> >> +       /* Check whether the MSI feature has been enabled by the PCI host. */
> >> +       flags = cdns_pcie_ep_fn_readw(pcie, fn, cap + PCI_MSI_FLAGS);
> >> +       if (!(flags & PCI_MSI_FLAGS_ENABLE))
> >> +               return -EINVAL;
> >> +
> >> +       /* Get the number of enabled MSIs */
> >> +       mme = (flags & PCI_MSI_FLAGS_QSIZE) >> 4;
> >> +       msi_count = 1 << mme;
> >> +       if (!interrupt_num || interrupt_num > msi_count)
> >> +               return -EINVAL;
> >> +
> >> +       /* Compute the data value to be written. */
> >> +       data_mask = msi_count - 1;
> >> +       data = cdns_pcie_ep_fn_readw(pcie, fn, cap + PCI_MSI_DATA_64);
> >> +       data = data & ~data_mask;
> >> +
> >> +       /* Get the PCI address where to write the data into. */
> >> +       pci_addr = cdns_pcie_ep_fn_readl(pcie, fn, cap + PCI_MSI_ADDRESS_HI);
> >> +       pci_addr <<= 32;
> >> +       pci_addr |= cdns_pcie_ep_fn_readl(pcie, fn, cap + PCI_MSI_ADDRESS_LO);
> >> +       pci_addr &= GENMASK_ULL(63, 2);
> >
> > Wouldn't all of the above be the same code for any endpoint driver? We
> > just need endpoint config space accessors for the same 32-bit only
> > access issues. Not asking for that in this series, but if that's the
> > direction we should go.
>
> Do you mean "endpoint" variant of pci_generic_config_read() which takes
> function number and capability offset? That could be done but we have to
> add support to traverse the linked list of capabilities though the
> capabilities are going to be at a fixed location for a given IP.

Well, the above code would call the equivalent of
pci_bus_read_config_*() functions which then calls driver specific
read/write ops like pci_generic_config_read().

Once we have common accessors, then functions to get the capability
offsets would be common too. It shouldn't matter that they happen to
be fixed, walking the linked list should work either way. Getting rid
of fixed offsets for the host side drivers is something I've been
doing too.

> Also in some cases, the writes are to a different register than the
> configuration space registers like vendor_id in Cadence EP should be
> written to Local Management register instead of the configuration space
> register.

We have the same issue on the host side as well. That just means we
need to wrap the generic ops functions.

Rob

  reply	other threads:[~2021-01-04 15:15 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-11 15:35 [PATCH v8 00/18] Implement NTB Controller using multiple PCI EP Kishon Vijay Abraham I
2020-11-11 15:35 ` [PATCH v8 01/18] Documentation: PCI: Add specification for the *PCI NTB* function device Kishon Vijay Abraham I
2020-11-11 15:35 ` [PATCH v8 02/18] PCI: endpoint: Make *_get_first_free_bar() take into account 64 bit BAR Kishon Vijay Abraham I
2020-11-11 15:35 ` [PATCH v8 03/18] PCI: endpoint: Add helper API to get the 'next' unreserved BAR Kishon Vijay Abraham I
2020-11-11 15:35 ` [PATCH v8 04/18] PCI: endpoint: Make *_free_bar() to return error codes on failure Kishon Vijay Abraham I
2020-11-11 15:35 ` [PATCH v8 05/18] PCI: endpoint: Remove unused pci_epf_match_device() Kishon Vijay Abraham I
2020-11-11 15:35 ` [PATCH v8 06/18] PCI: endpoint: Add support to associate secondary EPC with EPF Kishon Vijay Abraham I
2020-11-11 15:35 ` [PATCH v8 07/18] PCI: endpoint: Add support in configfs to associate two EPCs " Kishon Vijay Abraham I
2020-11-11 15:35 ` [PATCH v8 08/18] PCI: endpoint: Add pci_epc_ops to map MSI irq Kishon Vijay Abraham I
2020-11-11 15:35 ` [PATCH v8 09/18] PCI: endpoint: Add pci_epf_ops for epf drivers to expose function specific attrs Kishon Vijay Abraham I
2020-11-11 15:35 ` [PATCH v8 10/18] PCI: endpoint: Allow user to create sub-directory of 'EPF Device' directory Kishon Vijay Abraham I
2020-11-11 15:35 ` [PATCH v8 11/18] PCI: cadence: Implement ->msi_map_irq() ops Kishon Vijay Abraham I
2020-12-14 17:56   ` Tom Joseph
2020-12-15 16:01   ` Rob Herring
2021-01-04 13:12     ` Kishon Vijay Abraham I
2021-01-04 15:15       ` Rob Herring [this message]
2020-11-11 15:35 ` [PATCH v8 12/18] PCI: cadence: Configure LM_EP_FUNC_CFG based on epc->function_num_map Kishon Vijay Abraham I
2020-12-14 17:56   ` Tom Joseph
2020-11-11 15:35 ` [PATCH v8 13/18] PCI: endpoint: Add EP function driver to provide NTB functionality Kishon Vijay Abraham I
2020-11-11 15:35 ` [PATCH v8 14/18] PCI: Add TI J721E device to pci ids Kishon Vijay Abraham I
2020-11-11 15:35 ` [PATCH v8 15/18] NTB: Add support for EPF PCI-Express Non-Transparent Bridge Kishon Vijay Abraham I
2020-12-07 15:57   ` Jiang, Dave
2020-12-08  4:43     ` Kishon Vijay Abraham I
2020-11-11 15:35 ` [PATCH v8 16/18] NTB: tool: Enable the NTB/PCIe link on the local or remote side of bridge Kishon Vijay Abraham I
2020-12-07 15:55   ` Jiang, Dave
2020-12-08  4:46     ` Kishon Vijay Abraham I
2020-11-11 15:35 ` [PATCH v8 17/18] Documentation: PCI: Add configfs binding documentation for pci-ntb endpoint function Kishon Vijay Abraham I
2020-11-11 15:35 ` [PATCH v8 18/18] Documentation: PCI: Add userguide for PCI endpoint NTB function Kishon Vijay Abraham I

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='CAL_JsqKT8WUVy4qhQkRuYLuqkQa11=7JzXcVxvNRsB0KFj+qVQ@mail.gmail.com' \
    --to=robh@kernel.org \
    --cc=allenbh@gmail.com \
    --cc=arnd@arndb.de \
    --cc=bhelgaas@google.com \
    --cc=corbet@lwn.net \
    --cc=dave.jiang@intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jdmason@kudzu.us \
    --cc=kishon@ti.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-ntb@googlegroups.com \
    --cc=linux-pci@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=tjoseph@cadence.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 read-only IMAP folder(s) and NNTP newsgroup(s).