linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: Jingoo Han <jingoohan1@gmail.com>,
	Pratyush Anand <pratyush.anand@gmail.com>
Cc: linux-pci@vger.kernel.org, Jisheng Zhang <jszhang@marvell.com>,
	Minghuan.Lian@freescale.com
Subject: DesignWare ATU PCIE_ATU_TYPE_MEM usage
Date: Tue, 5 Jan 2016 16:42:22 -0600	[thread overview]
Message-ID: <20160105224222.GB23814@localhost> (raw)

Hi guys,

This code in dw_pcie_host_init() looks wrong:

    if (!pp->ops->rd_other_conf)
	    dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX1,
				      PCIE_ATU_TYPE_MEM, pp->mem_base,
				      pp->mem_bus_addr, pp->mem_size);

    dw_pcie_wr_own_conf(pp, PCI_BASE_ADDRESS_0, 4, 0);
    ...
    dw_pcie_rd_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4, &val);

Evidently you need to program the ATU with PCIE_ATU_TYPE_MEM before
doing config accesses on the root bus?

If that's the case, what about other config accesses after these few
in dw_pcie_host_init()?  I don't see anything that changes the ATU
programming for things coming through dw_pcie_wr_conf().

I assume you need some sort of locking around this sequence:

    dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0, type, ...
    ret = dw_pcie_cfg_read(va_cfg_base + where, size, val);
    dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0, PCIE_ATU_TYPE_IO, ...

Are you relying on pci_lock?  If so, a comment to that effect would be
nice.

The whole ATU management looks pretty inefficient.  It's likely that
there'll be a whole sequence of operations where we don't need to
touch the ATU, but since we don't remember its current state, we
configure the whole thing from scratch each time, which is at least
eight register writes (plus a read for the new synchronization).

Bjorn

             reply	other threads:[~2016-01-05 22:42 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-05 22:42 Bjorn Helgaas [this message]
2016-01-07  7:11 ` DesignWare ATU PCIE_ATU_TYPE_MEM usage Minghuan Lian
2016-01-07  8:32   ` Jisheng Zhang
2016-01-07 10:20   ` Pratyush Anand

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=20160105224222.GB23814@localhost \
    --to=helgaas@kernel.org \
    --cc=Minghuan.Lian@freescale.com \
    --cc=jingoohan1@gmail.com \
    --cc=jszhang@marvell.com \
    --cc=linux-pci@vger.kernel.org \
    --cc=pratyush.anand@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).