Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: Vidya Sagar <vidyas@nvidia.com>
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
	lorenzo.pieralisi@arm.com, mperttunen@nvidia.com,
	mmaddireddy@nvidia.com, linux-pci@vger.kernel.org,
	catalin.marinas@arm.com, will.deacon@arm.com,
	linux-kernel@vger.kernel.org, robh+dt@kernel.org, kishon@ti.com,
	kthota@nvidia.com, thierry.reding@gmail.com,
	gustavo.pimentel@synopsys.com, jingoohan1@gmail.com,
	linux-tegra@vger.kernel.org, jonathanh@nvidia.com,
	linux-arm-kernel@lists.infradead.org, sagar.tv@gmail.com
Subject: Re: [PATCH V7 04/15] PCI: dwc: Move config space capability search API
Date: Wed, 22 May 2019 09:02:36 -0500
Message-ID: <20190522140235.GB79339@google.com> (raw)
In-Reply-To: <fd164d1f-cf99-fe81-c368-46e3a3742a59@nvidia.com>

On Wed, May 22, 2019 at 02:26:08PM +0530, Vidya Sagar wrote:
> On 5/22/2019 2:47 AM, Bjorn Helgaas wrote:
> > On Fri, May 17, 2019 at 06:08:35PM +0530, Vidya Sagar wrote:
> > > Move PCIe config space capability search API to common DesignWare file
> > > as this can be used by both host and ep mode codes.

> > >   .../pci/controller/dwc/pcie-designware-ep.c   | 37 +----------------
> > >   drivers/pci/controller/dwc/pcie-designware.c  | 40 +++++++++++++++++++
> > >   drivers/pci/controller/dwc/pcie-designware.h  |  2 +
> > >   3 files changed, 44 insertions(+), 35 deletions(-)

> > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > @@ -14,6 +14,46 @@
> > >   #include "pcie-designware.h"
> > > +/*
> > > + * These APIs are different from standard pci_find_*capability() APIs in the
> > > + * sense that former can only be used post device enumeration as they require
> > > + * 'struct pci_dev *' pointer whereas these APIs require 'struct dw_pcie *'
> > > + * pointer and can be used before link up also.
> > 
> > I think this comment is slightly misleading because it suggests the
> > reason we need these DW interfaces is because we're doing something
> > before a pci_dev pointer is available.
> > 
> > But these DW interfaces are used on devices that will *never* have a
> > pci_dev pointer because they are not PCI devices.  They're used on
> > host controller devices, which have a PCIe link on the downstream
> > side, but the host controller driver operates them using their
> > upstream, non-PCI interfaces.  Logically, I think they would be
> > considered parts of Root Complexes, not Root Ports.
> > 
> > There's actually no reason why that upstream interface should look
> > anything like PCI; it doesn't need to organize registers into
> > capability lists at all.  It might be convenient for the hardware to
> > do that and share things with a Root Port device, which *is* a PCI
> > device, but it's not required.
> > 
> > It also really has nothing to do with whether the link is up.  This
> > code operates on hardware that is upstream from the link, so we can
> > reach it regardless of the link.
>
> I added this comment after receiving a review comment to justify why
> standard pci_find_*capability() APIs can't be used here. Hence added
> this.  I understand your comment that DW interface need not have to
> be a PCI device, but what is present in the hardware is effectively
> a root port implementation and post enumeration, we get a 'struct
> pci_dev' created for it, hence I thought it is fine to bring 'struct
> pci_dev' into picture.

This code operates on the host controller.  It configures the bridge
that leads *to* PCI devices.  Since that bridge is not a PCI device,
the PCI specs don't say anything about how to program it.

The fact that the host controller programming interface happens to
resemble the PCI programming interface is purely coincidental.

> Also, I agree that mention of 'link up' is unwarranted and could be
> reworded in a better way.
>
> Do you suggest to remove this comment altogether or reword it s/and
> can be used before link up also/and can be used before 'struct
> pci_dev' is available/ ?

Maybe something like this?

    These interfaces resemble the pci_find_*capability() interfaces,
    but these are for configuring host controllers, which are bridges
    *to* PCI devices but are not PCI devices themselves.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply index

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-17 12:38 [PATCH V7 00/15] Add Tegra194 PCIe support Vidya Sagar
2019-05-17 12:38 ` [PATCH V7 01/15] PCI: Add #defines for some of PCIe spec r4.0 features Vidya Sagar
2019-05-17 12:38 ` [PATCH V7 02/15] PCI: Disable MSI for Tegra194 root port Vidya Sagar
2019-05-21 10:27   ` Thierry Reding
2019-05-21 16:47     ` Vidya Sagar
2019-05-21 19:34       ` Vidya Sagar
2019-05-21 19:36       ` Bjorn Helgaas
2019-05-22  8:07         ` Vidya Sagar
2019-05-17 12:38 ` [PATCH V7 03/15] PCI: dwc: Perform dbi regs write lock towards the end Vidya Sagar
2019-05-21 10:29   ` Thierry Reding
2019-05-17 12:38 ` [PATCH V7 04/15] PCI: dwc: Move config space capability search API Vidya Sagar
2019-05-21 10:29   ` Thierry Reding
2019-05-21 21:17   ` Bjorn Helgaas
2019-05-22  8:56     ` Vidya Sagar
2019-05-22 14:02       ` Bjorn Helgaas [this message]
2019-05-24 14:46         ` Vidya Sagar
2019-05-17 12:38 ` [PATCH V7 05/15] PCI: dwc: Add ext " Vidya Sagar
2019-05-21 10:36   ` Thierry Reding
2019-05-21 17:14     ` Vidya Sagar
2019-05-17 12:38 ` [PATCH V7 06/15] dt-bindings: PCI: designware: Add binding for CDM register check Vidya Sagar
2019-05-21 10:37   ` Thierry Reding
2019-05-24 20:23   ` Rob Herring
2019-05-17 12:38 ` [PATCH V7 07/15] PCI: dwc: Add support to enable " Vidya Sagar
2019-05-21 10:38   ` Thierry Reding
2019-05-17 12:38 ` [PATCH V7 08/15] dt-bindings: Add PCIe supports-clkreq property Vidya Sagar
2019-05-21 10:39   ` Thierry Reding
2019-05-17 12:38 ` [PATCH V7 09/15] dt-bindings: PCI: tegra: Add device tree support for Tegra194 Vidya Sagar
2019-05-21 10:51   ` Thierry Reding
2019-05-21 18:00     ` Vidya Sagar
2019-05-24 20:26   ` Rob Herring
2019-05-17 12:38 ` [PATCH V7 10/15] dt-bindings: PHY: P2U: Add Tegra194 P2U block Vidya Sagar
2019-05-21 10:52   ` Thierry Reding
2019-05-17 12:38 ` [PATCH V7 11/15] arm64: tegra: Add P2U and PCIe controller nodes to Tegra194 DT Vidya Sagar
2019-05-17 13:03   ` Ard Biesheuvel
2019-05-17 17:38     ` Vidya Sagar
2019-05-17 12:38 ` [PATCH V7 12/15] arm64: tegra: Enable PCIe slots in P2972-0000 board Vidya Sagar
2019-05-21 10:54   ` Thierry Reding
2019-05-21 18:17     ` Vidya Sagar
2019-05-22 13:48       ` Thierry Reding
2019-05-17 12:38 ` [PATCH V7 13/15] phy: tegra: Add PCIe PIPE2UPHY support Vidya Sagar
2019-05-21 11:00   ` Thierry Reding
2019-05-21 19:37     ` Vidya Sagar
2019-05-21 11:00   ` Thierry Reding
2019-05-22  8:59     ` Vidya Sagar
2019-05-17 12:38 ` [PATCH V7 14/15] PCI: tegra: Add Tegra194 PCIe support Vidya Sagar
2019-05-21 11:41   ` Thierry Reding
2019-05-22 12:05     ` Vidya Sagar
2019-05-22 14:14       ` Thierry Reding
2019-05-24 18:07         ` Vidya Sagar
2019-05-17 12:38 ` [PATCH V7 15/15] arm64: Add Tegra194 PCIe driver to defconfig Vidya Sagar

Reply instructions:

You may reply publically 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=20190522140235.GB79339@google.com \
    --to=helgaas@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=devicetree@vger.kernel.org \
    --cc=gustavo.pimentel@synopsys.com \
    --cc=jingoohan1@gmail.com \
    --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=mark.rutland@arm.com \
    --cc=mmaddireddy@nvidia.com \
    --cc=mperttunen@nvidia.com \
    --cc=robh+dt@kernel.org \
    --cc=sagar.tv@gmail.com \
    --cc=thierry.reding@gmail.com \
    --cc=vidyas@nvidia.com \
    --cc=will.deacon@arm.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

Linux-ARM-Kernel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/0 linux-arm-kernel/git/0.git
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/1 linux-arm-kernel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-kernel linux-arm-kernel/ https://lore.kernel.org/linux-arm-kernel \
		linux-arm-kernel@lists.infradead.org
	public-inbox-index linux-arm-kernel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-arm-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git