From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
To: "Pali Rohár" <pali@kernel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
Rob Herring <robh+dt@kernel.org>,
Mauri Sandberg <maukka@ext.kapsi.fi>,
devicetree@vger.kernel.org, linux-pci@vger.kernel.org
Subject: Re: How to correctly define memory range of PCIe config space
Date: Sat, 6 Aug 2022 17:46:14 +0530 [thread overview]
Message-ID: <20220806121614.GA11359@thinkpad> (raw)
In-Reply-To: <20220806111702.ezzknr76a4imej4u@pali>
On Sat, Aug 06, 2022 at 01:17:02PM +0200, Pali Rohár wrote:
> On Saturday 06 August 2022 16:36:13 Manivannan Sadhasivam wrote:
> > Hi Pali,
> >
> > On Mon, Jul 11, 2022 at 12:51:08AM +0200, Pali Rohár wrote:
> > > Hello!
> > >
> > > Together with Mauri we are working on extending pci-mvebu.c driver to
> > > support Orion PCIe controllers as these controllers are same as mvebu
> > > controller.
> > >
> > > There is just one big difference: Config space access on Orion is
> > > different. mvebu uses classic Intel CFC/CF8 registers for indirect
> > > config space access but Orion has direct memory mapped config space.
> > > So Orion DTS files need to have this memory range for config space and
> > > pci-mvebu.c driver have to read this range from DTS and properly map it.
> > >
> > > So my question is: How to properly define config space range in device
> > > tree file? In which device tree property and in which format? Please
> > > note that this memory range of config space is PCIe root port specific
> > > and it requires its own MBUS_ID() like memory range of PCIe MEM and PCIe
> > > IO mapping. Please look e.g. at armada-385.dtsi how are MBUS_ID() used:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/armada-385.dtsi
> > >
> >
> > On most of the platforms, the standard "reg" property is used to specify the
> > config space together with other device specific memory regions. For instance,
> > on the Qcom platforms based on Designware IP, we have below regions:
> >
> > reg = <0xfc520000 0x2000>,
> > <0xff000000 0x1000>,
> > <0xff001000 0x1000>,
> > <0xff002000 0x2000>;
> > reg-names = "parf", "dbi", "elbi", "config";
> >
> > Where "parf" and "elbi" are Qcom controller specific regions, while "dbi" and
> > "config" (config space) are common to all Designware IPs.
> >
> > These properties are documented in: Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> >
> > Hope this helps!
>
> Hello! I have already looked at this. But as I pointed in above
> armada-385.dtsi file, mvebu is quite complicated. First it does not use
> explicit address ranges, but rather macros MBUS_ID() which assign
> addresses at kernel runtime by mbus driver. Second issue is that config
> space range (like any other resources) are pcie root port specific. So
> it cannot be in pcie controller node and in pcie devices is "reg"
> property reserved for pci bdf address.
>
> In last few days, I spent some time on this issue and after reading lot
> of pcie dts files, including bindings and other documents (including
> open firmware pci2_1.pdf) and I'm proposing following definition:
>
> soc {
> pcie-mem-aperture = <0xe0000000 0x08000000>; /* 128 MiB memory space */
> pcie-cfg-aperture = <0xf0000000 0x01000000>; /* 16 MiB config space */
> pcie-io-aperture = <0xf2000000 0x00100000>; /* 1 MiB I/O space */
>
> pcie {
> ranges = <0x82000000 0 0x40000 MBUS_ID(0xf0, 0x01) 0x40000 0x0 0x2000>, /* Port 0.0 Internal registers */
> <0x82000000 0 0xf0000000 MBUS_ID(0x04, 0x79) 0 0x0 0x1000000>, /* Port 0.0 Config space */
> <0x82000000 1 0x0 MBUS_ID(0x04, 0x59) 0 0x1 0x0>, /* Port 0.0 Mem */
> <0x81000000 1 0x0 MBUS_ID(0x04, 0x51) 0 0x1 0x0>, /* Port 0.0 I/O */
>
> pcie@1,0 {
> reg = <0x0800 0 0 0 0>; /* BDF 0:1.0 */
> assigned-addresses = <0x82000800 0 0x40000 0x0 0x2000>, /* Port 0.0 Internal registers */
> <0x82000800 0 0xf0000000 0x0 0x1000000>; /* Port 0.0 Config space */
> ranges = <0x82000000 0 0 0x82000000 1 0 0x1 0x0>, /* Port 0.0 Mem */
> 0x81000000 0 0 0x81000000 1 0 0x1 0x0>; /* Port 0.0 I/O */
> };
> };
> };
>
> So the pci config space address range would be defined in
> "assigned-addresses" property as the _second_ value. First value is
> already used for specifying internal registers (similar what is "parf"
> for qcom).
>
Sounds reasonable to me. Another option would be to introduce a mvebu specific
property but that would be the least preferred option I guess.
But the fact that "assigned-addresses" property is described as "MMIO registers"
also adds up to the justification IMO.
Rob/Krzysztof could always correct that during binding review.
> config space is currently limited to 16 MB (without extended PCIe), but
> after we find free continuous physical address window of size 256MB we
> can extend it to full PCIe config space range.
>
> Any objections to above device tree definition?
>
Are you also converting the binding to YAML for validation?
Thanks,
Mani
> > Thanks,
> > Mani
> >
> > > Krzysztof, would you be able to help with proper definition of this
> > > property, so it would be fine also for schema checkers or other
> > > automatic testing tools?
> >
> > --
> > மணிவண்ணன் சதாசிவம்
--
மணிவண்ணன் சதாசிவம்
next prev parent reply other threads:[~2022-08-06 12:17 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-10 22:51 How to correctly define memory range of PCIe config space Pali Rohár
2022-07-23 9:05 ` Pali Rohár
2022-08-05 12:45 ` Pali Rohár
2022-08-06 11:06 ` Manivannan Sadhasivam
2022-08-06 11:17 ` Pali Rohár
2022-08-06 12:16 ` Manivannan Sadhasivam [this message]
2022-08-06 12:23 ` Pali Rohár
2022-08-06 12:46 ` Manivannan Sadhasivam
2022-08-09 16:13 ` Rob Herring
2022-08-09 15:59 ` Rob Herring
2022-08-09 16:29 ` Pali Rohár
2022-08-09 17:06 ` Rob Herring
2022-08-09 17:47 ` Pali Rohár
2022-09-05 17:02 ` Pali Rohár
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=20220806121614.GA11359@thinkpad \
--to=manivannan.sadhasivam@linaro.org \
--cc=devicetree@vger.kernel.org \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=linux-pci@vger.kernel.org \
--cc=maukka@ext.kapsi.fi \
--cc=pali@kernel.org \
--cc=robh+dt@kernel.org \
/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).