From: Stuart Yoder <b08248@gmail.com>
To: Varun Sethi <Varun.Sethi@freescale.com>
Cc: Joerg Roedel <joro@8bytes.org>,
Stuart Yoder <stuart.yoder@freescale.com>,
linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org,
Scott Wood <scottwood@freescale.com>,
linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH 6/6 v8] iommu/fsl: Freescale PAMU driver and IOMMU API implementation.
Date: Tue, 26 Feb 2013 16:33:15 -0600 [thread overview]
Message-ID: <CALRxmdBBO2AoAnKSTNNTYhHZ9fitJKWWeZMUj1ObtFhsZS5iSw@mail.gmail.com> (raw)
In-Reply-To: <1361191939-21260-7-git-send-email-Varun.Sethi@freescale.com>
Have not got through the entire file, but have a few comments...
+/*
+ * Set the PAACE type as primary and set the coherency required domain
+ * attribute
+ */
+static void pamu_setup_default_xfer_to_host_ppaace(struct paace *ppaace)
+{
+ set_bf(ppaace->addr_bitfields, PAACE_AF_PT, PAACE_PT_PRIMARY);
+
+ set_bf(ppaace->domain_attr.to_host.coherency_required, PAACE_DA_HOST_CR,
+ PAACE_M_COHERENCE_REQ);
+}
+
+/*
+ * Set the PAACE type as secondary and set the coherency required domain
+ * attribute.
+ */
+static void pamu_setup_default_xfer_to_host_spaace(struct paace *spaace)
+{
+ set_bf(spaace->addr_bitfields, PAACE_AF_PT, PAACE_PT_SECONDARY);
+ set_bf(spaace->domain_attr.to_host.coherency_required, PAACE_DA_HOST_CR,
+ PAACE_M_COHERENCE_REQ);
+}
Can we change the names of the above functions... I know there is some history
with the name, but "xfer_to_host" is confusing.
Maybe just call them:
pamu_init_paace()
pamu_init_spaace()
> +/**
> + * pamu_config_spaace() - Sets up SPAACE entry for specified subwindow
> + *
> + * @liodn: Logical IO device number
> + * @subwin_cnt: number of sub-windows associated with dma-window
> + * @subwin_addr: starting address of subwindow
> + * @subwin_size: size of subwindow
> + * @omi: Operation mapping index
> + * @rpn: real (true physical) page number
> + * @snoopid: snoop id for hardware coherency -- if ~snoopid == 0 then
> + * snoopid not defined
> + * @stashid: cache stash id for associated cpu
> + * @enable: enable/disable subwindow after reconfiguration
> + * @prot: sub window permissions
> + *
> + * Returns 0 upon success else error code < 0 returned
> + */
> +int pamu_config_spaace(int liodn, u32 subwin_cnt, u32 subwin_addr,
> + phys_addr_t subwin_size, u32 omi, unsigned long rpn,
> + u32 snoopid, u32 stashid, int enable, int prot)
> +{
> + struct paace *paace;
> +
> + /* setup sub-windows */
> + if (!subwin_cnt) {
> + pr_err("Invalid subwindow count\n");
> + return -EINVAL;
> + }
> +
> + paace = pamu_get_ppaace(liodn);
> + if (subwin_addr > 0 && subwin_addr < subwin_cnt && paace) {
Why is the comparison subwin_addr < subwin_cnt? Seems wrong...
> + paace = pamu_get_spaace(paace, subwin_addr - 1);
> +
> + if (paace && !(paace->addr_bitfields & PAACE_V_VALID)) {
> + pamu_setup_default_xfer_to_host_spaace(paace);
> + set_bf(paace->addr_bitfields, SPAACE_AF_LIODN, liodn);
> + }
> + }
> +
> + if (!paace) {
> + pr_err("Invalid liodn entry\n");
> + return -ENOENT;
> + }
> +
> + if (!enable && prot == PAACE_AP_PERMS_DENIED) {
> + if (subwin_addr > 0)
> + set_bf(paace->addr_bitfields, PAACE_AF_V,
> + PAACE_V_INVALID);
> + else
> + set_bf(paace->addr_bitfields, PAACE_AF_AP,
> + prot);
> + mb();
> + return 0;
> + }
Can you add a comment to the above if statement...when is this function called
with PAACE_AP_PERMS_DENIED?
> + if (subwin_size & (subwin_size - 1) || subwin_size < PAMU_PAGE_SIZE) {
> + pr_err("subwindow size out of range, or not a power of 2\n");
> + return -EINVAL;
> + }
> +
> + if (rpn == ULONG_MAX) {
> + pr_err("real page number out of range\n");
> + return -EINVAL;
> + }
> +
> + /* window size is 2^(WSE+1) bytes */
> + set_bf(paace->win_bitfields, PAACE_WIN_SWSE,
> + map_addrspace_size_to_wse(subwin_size));
> +
> + set_bf(paace->impl_attr, PAACE_IA_ATM, PAACE_ATM_WINDOW_XLATE);
> + paace->twbah = rpn >> 20;
> + set_bf(paace->win_bitfields, PAACE_WIN_TWBAL, rpn);
> + set_bf(paace->addr_bitfields, PAACE_AF_AP, prot);
> +
> + /* configure snoop id */
> + if (~snoopid != 0)
> + paace->domain_attr.to_host.snpid = snoopid;
> +
> + /* set up operation mapping if it's configured */
> + if (omi < OME_NUMBER_ENTRIES) {
> + set_bf(paace->impl_attr, PAACE_IA_OTM, PAACE_OTM_INDEXED);
> + paace->op_encode.index_ot.omi = omi;
> + } else if (~omi != 0) {
> + pr_err("bad operation mapping index: %d\n", omi);
> + return -EINVAL;
> + }
> +
> + if (~stashid != 0)
> + set_bf(paace->impl_attr, PAACE_IA_CID, stashid);
> +
> + smp_wmb();
> +
> + if (enable)
> + paace->addr_bitfields |= PAACE_V_VALID;
> +
> + mb();
> +
> + return 0;
> +}
next prev parent reply other threads:[~2013-02-26 22:33 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-02-18 12:52 [PATCH 0/6 v8] iommu/fsl: Freescale PAMU driver and IOMMU API implementation Varun Sethi
2013-02-18 12:52 ` [PATCH 1/6 v8] iommu/fsl: Store iommu domain information pointer in archdata Varun Sethi
2013-02-27 11:30 ` Joerg Roedel
2013-02-27 12:04 ` Sethi Varun-B16395
2013-02-28 15:51 ` Kumar Gala
2013-03-01 1:24 ` Alexey Kardashevskiy
2013-03-01 8:55 ` Sethi Varun-B16395
2013-03-01 10:07 ` Alexey Kardashevskiy
2013-03-01 16:21 ` Yoder Stuart-B08248
2013-03-04 6:35 ` Sethi Varun-B16395
2013-02-18 12:52 ` [PATCH 2/6] powerpc/fsl_pci: Store the platform device information corresponding to the pci controller Varun Sethi
2013-02-26 0:09 ` Stuart Yoder
2013-02-26 6:16 ` Sethi Varun-B16395
2013-02-27 10:45 ` Joerg Roedel
2013-02-27 10:56 ` Sethi Varun-B16395
2013-02-28 15:45 ` Kumar Gala
2013-03-07 9:14 ` Sethi Varun-B16395
2013-03-07 10:37 ` Joerg Roedel
2013-02-18 12:52 ` [PATCH 3/6] powerpc/fsl_pci: Added defines for the FSL PCI controller BRR1 register Varun Sethi
2013-02-27 11:33 ` Joerg Roedel
2013-02-28 15:46 ` Kumar Gala
2013-02-18 12:52 ` [PATCH 4/6] iommu/fsl: Add window permission flags for iommu_domain_window_enable API Varun Sethi
2013-02-18 12:52 ` [PATCH 5/6 v8] iommu/fsl: Add addtional attributes specific to the PAMU driver Varun Sethi
2013-02-27 11:38 ` Joerg Roedel
2013-02-18 12:52 ` [PATCH 6/6 v8] iommu/fsl: Freescale PAMU driver and IOMMU API implementation Varun Sethi
2013-02-19 10:04 ` Diana Craciun
2013-02-19 10:27 ` Sethi Varun-B16395
2013-02-19 15:59 ` Diana Craciun
2013-02-20 9:41 ` Sethi Varun-B16395
2013-02-26 22:33 ` Stuart Yoder [this message]
2013-02-27 11:56 ` Sethi Varun-B16395
2013-02-28 0:03 ` Stuart Yoder
2013-03-01 23:27 ` Stuart Yoder
2013-03-04 11:31 ` Sethi Varun-B16395
2013-03-04 16:55 ` Yoder Stuart-B08248
2013-02-25 10:15 ` [PATCH 0/6 " Sethi Varun-B16395
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=CALRxmdBBO2AoAnKSTNNTYhHZ9fitJKWWeZMUj1ObtFhsZS5iSw@mail.gmail.com \
--to=b08248@gmail.com \
--cc=Varun.Sethi@freescale.com \
--cc=iommu@lists.linux-foundation.org \
--cc=joro@8bytes.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=scottwood@freescale.com \
--cc=stuart.yoder@freescale.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).