From: Rob Herring <robh@kernel.org>
To: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Cc: Linuxarm <linuxarm@huawei.com>,
mauro.chehab@huawei.com, Binghui Wang <wangbinghui@hisilicon.com>,
Gustavo Pimentel <gustavo.pimentel@synopsys.com>,
Jingoo Han <jingoohan1@gmail.com>,
Xiaowei Song <songxiaowei@hisilicon.com>,
devicetree@vger.kernel.org,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
PCI <linux-pci@vger.kernel.org>,
linux-phy@lists.infradead.org
Subject: Re: [PATCH v3 0/4] DT schema changes for HiKey970 PCIe hardware to work
Date: Tue, 10 Aug 2021 11:52:34 -0600 [thread overview]
Message-ID: <CAL_JsqKr9csV5fPZxD=kRRB5W6RCjHz0VsP6-nx0RQt8mgVJ5w@mail.gmail.com> (raw)
In-Reply-To: <CAL_JsqL-R=kTugNAC-C1gfSm6Xnb0Nw_iLcRki8aQMNQjcLN6A@mail.gmail.com>
On Tue, Aug 10, 2021 at 11:13 AM Rob Herring <robh@kernel.org> wrote:
>
> On Tue, Aug 10, 2021 at 8:21 AM Mauro Carvalho Chehab
> <mchehab+huawei@kernel.org> wrote:
> >
> > Em Tue, 10 Aug 2021 07:44:50 -0600
> > Rob Herring <robh@kernel.org> escreveu:
> >
> > > On Tue, Aug 10, 2021 at 3:42 AM Mauro Carvalho Chehab
> > > <mchehab+huawei@kernel.org> wrote:
> > > >
> > > > Em Fri, 6 Aug 2021 10:23:35 -0600
> > > > Rob Herring <robh@kernel.org> escreveu:
> > > >
> > > > > On Thu, Aug 5, 2021 at 1:58 AM Mauro Carvalho Chehab
> > > > > <mchehab+huawei@kernel.org> wrote:
> > > > > >
> > > > > > Em Thu, 5 Aug 2021 09:46:12 +0200
> > > > > > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:
> > > > > >
> > > > > > > Em Wed, 4 Aug 2021 10:28:53 -0600
> > > > > > > Rob Herring <robh@kernel.org> escreveu:
> > > > > > >
> > > > > > > > On Wed, Aug 04, 2021 at 08:50:45AM +0200, Mauro Carvalho Chehab wrote:
> > > > > > > > > Em Tue, 3 Aug 2021 16:11:42 -0600
> > > > > > > > > Rob Herring <robh+dt@kernel.org> escreveu:
> > > > > > > > >
> > > > > > > > > > On Mon, Aug 2, 2021 at 10:39 PM Mauro Carvalho Chehab
> > > > > > > > > > <mchehab+huawei@kernel.org> wrote:
> > > > > > > > > > >
> > > > > > > > > > > Hi Rob,
> > > > > > > > > > >
> > > > > > > > > > > That's the third version of the DT bindings for Kirin 970 PCIE and its
> > > > > > > > > > > corresponding PHY.
> > > > > > > > > > >
> > > > > > > > > > > It is identical to v2, except by:
> > > > > > > > > > > - pcie@7,0 { // Lane 7: Ethernet
> > > > > > > > > > > + pcie@7,0 { // Lane 6: Ethernet
> > > > > > > > > >
> > > > > > > > > > Can you check whether you have DT node links in sysfs for the PCI
> > > > > > > > > > devices? If you don't, then something is wrong still in the topology
> > > > > > > > > > or the PCI core is failing to set the DT node pointer in struct
> > > > > > > > > > device. Though you don't rely on that currently, we want the topology
> > > > > > > > > > to match. It's possible this never worked on arm/arm64 as mainly
> > > > > > > > > > powerpc relied on this.
> > > > > > > > > >
> > > > > > > > > > I'd like some way to validate the DT matches the PCI topology. We
> > > > > > > > > > could have a tool that generates the DT structure based on the PCI
> > > > > > > > > > topology.
> > > > > > > > >
> > > > > > > > > The of_node node link is on those places:
> > > > > > > > >
> > > > > > > > > $ find /sys/devices/platform/soc/f4000000.pcie/ -name of_node
> > > > > > > > > /sys/devices/platform/soc/f4000000.pcie/of_node
> > > > > > > > > /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/of_node
> > > > > > > > > /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/pci_bus/0000:01/of_node
> > > > > > > > > /sys/devices/platform/soc/f4000000.pcie/pci0000:00/pci_bus/0000:00/of_node
> > > > > > > >
> > > > > > > > Looks like we're missing some...
> > > > > > > >
> > > > > > > > It's not immediately obvious to me what's wrong here. Only the root
> > > > > > > > bus is getting it's DT node set. The relevant code is pci_scan_device(),
> > > > > > > > pci_set_of_node() and pci_set_bus_of_node(). Give me a few days to try
> > > > > > > > to reproduce and debug it.
> > > > > > >
> > > > > > > I added a printk on both pci_set_*of_node() functions:
> > > > > > >
> > > > > > > [ 4.872991] (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
> > > > > > > [ 4.913806] (null): pci_set_of_node: of_node: /soc/pcie@f4000000
> > > > > > > [ 4.978102] pci_bus 0000:01: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > > > > > > [ 4.990622] (null): pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > > > > > > [ 5.052383] pci_bus 0000:02: pci_set_bus_of_node: of_node: (null)
> > > > > > > [ 5.059263] (null): pci_set_of_node: of_node: (null)
> > > > > > > [ 5.085552] (null): pci_set_of_node: of_node: (null)
> > > > > > > [ 5.112073] (null): pci_set_of_node: of_node: (null)
> > > > > > > [ 5.138320] (null): pci_set_of_node: of_node: (null)
> > > > > > > [ 5.164673] (null): pci_set_of_node: of_node: (null)
> > > > > > > [ 5.233759] pci_bus 0000:03: pci_set_bus_of_node: of_node: (null)
> > > > > > > [ 5.240539] (null): pci_set_of_node: of_node: (null)
> > > > > > > [ 5.310545] pci_bus 0000:04: pci_set_bus_of_node: of_node: (null)
> > > > > > > [ 5.324719] pci_bus 0000:05: pci_set_bus_of_node: of_node: (null)
> > > > > > > [ 5.338914] pci_bus 0000:06: pci_set_bus_of_node: of_node: (null)
> > > > > > > [ 5.345516] (null): pci_set_of_node: of_node: (null)
> > > > > > > [ 5.415795] pci_bus 0000:07: pci_set_bus_of_node: of_node: (null)
> > > > > >
> > > > > > The enclosed patch makes the above a clearer:
> > > > > >
> > > > > > [ 4.800975] (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
> > > > > > [ 4.855983] pci 0000:00:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000
> > > > > > [ 4.879169] pci_bus 0000:01: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > > > > > [ 4.900602] pci 0000:01:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > > > > > [ 4.953086] pci_bus 0000:02: pci_set_bus_of_node: of_node: (null)
> > > > >
> > > > > I believe the issue is we need another bridge node in the DT
> > > > > hierarchy. What we have is:
> > > > >
> > > > > Bus 0 is node /soc/pcie@f4000000
> > > > > Bus 1 is device 0 on bus 0 is node /soc/pcie@f4000000/pcie@0,0
> > > > > Bus 2 is device 0 on bus 1 in node ... whoops, there's no device 0
> > > > > under /soc/pcie@f4000000/pcie@0,0
> > > > >
> > > > > So we need the hierarchy to be: /soc/pcie@f4000000/pcie@0/pcie@0/pcie@{1,5,7}
> > > >
> > > > Adding a child pcie@0 produces the following output from my debug
> > > > patches:
> > >
> > > You removed your changes to the PCI code other than the debug print?
> >
> > Yes.
> >
> > > >
> > > > [ 4.984278] (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
> > > > [ 5.042992] pci 0000:00:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000
> > > > [ 5.083738] pci_bus 0000:01: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > > > [ 5.124377] pci 0000:01:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > > > [ 5.168395] pci_bus 0000:02: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> > > > [ 5.200719] pci 0000:02:01.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> > >
> > > This should not happen. The devfn doesn't match.
> > >
> > > > [ 5.247777] pci 0000:02:04.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> > > > [ 5.276768] pci 0000:02:05.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> > > > [ 5.305018] pci 0000:02:07.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> > > > [ 5.333093] pci 0000:02:09.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> > > > [ 5.395620] pci_bus 0000:03: pci_set_bus_of_node: of_node: (null)
> > > > [ 5.416333] pci 0000:03:00.0: pci_set_of_node: of_node: (null)
> > > > [ 5.451353] pci_bus 0000:04: pci_set_bus_of_node: of_node: (null)
> > > > [ 5.473970] pci_bus 0000:05: pci_set_bus_of_node: of_node: (null)
> > > > [ 5.487765] pci_bus 0000:06: pci_set_bus_of_node: of_node: (null)
> > > > [ 5.530219] pci 0000:06:00.0: pci_set_of_node: of_node: (null)
> > > > [ 5.560896] pci_bus 0000:07: pci_set_bus_of_node: of_node: (null)
> > > >
> > > > It produces the following sysfs nodes:
> > > >
> > > > $ find /sys/devices/platform/soc/f4000000.pcie/ -name of_node
> > > > /sys/devices/platform/soc/f4000000.pcie/of_node
> > > > /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/of_node
> > > > /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/of_node
> > > > /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/pci_bus/0000:02/of_node
> > > > /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/pci_bus/0000:01/of_node
> > > > /sys/devices/platform/soc/f4000000.pcie/pci0000:00/pci_bus/0000:00/of_node
> > > >
> > > >
> > > > I'm enclosing the DT schema I'm using.
> > > >
> > > >
> > > >
> > > > Thanks,
> > > > Mauro
> > > >
> > > > ---
> > > >
> > > > pcie@f4000000 {
> > > > compatible = "hisilicon,kirin970-pcie";
> > > > reg = <0x0 0xf4000000 0x0 0x1000000>,
> > > > <0x0 0xfc180000 0x0 0x1000>,
> > > > <0x0 0xf5000000 0x0 0x2000>;
> > > > reg-names = "dbi", "apb", "config";
> > > > bus-range = <0x00 0xff>;
> > > > #address-cells = <3>;
> > > > #size-cells = <2>;
> > > > device_type = "pci";
> > > > phys = <&pcie_phy>;
> > > > ranges = <0x02000000 0x0 0x00000000
> > > > 0x0 0xf6000000
> > > > 0x0 0x02000000>;
> > > > num-lanes = <1>;
> > > > #interrupt-cells = <1>;
> > > > interrupts = <GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>;
> > > > interrupt-names = "msi";
> > > > interrupt-map-mask = <0 0 0 7>;
> > > > interrupt-map = <0x0 0 0 1
> > > > &gic GIC_SPI 282 IRQ_TYPE_LEVEL_HIGH>,
> > > > <0x0 0 0 2
> > > > &gic GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>,
> > > > <0x0 0 0 3
> > > > &gic GIC_SPI 284 IRQ_TYPE_LEVEL_HIGH>,
> > > > <0x0 0 0 4
> > > > &gic GIC_SPI 285 IRQ_TYPE_LEVEL_HIGH>;
> > > > reset-gpios = <&gpio7 0 0>;
> > > > hisilicon,clken-gpios = <&gpio27 3 0>, <&gpio17 0 0>,
> > > > <&gpio20 6 0>;
> > > > pcie@0,0 { // Lane 0: PCIe switch: Bus 1, Device 0
> > > > reg = <0x80 0 0 0 0>;
> > >
> > > s/0x80/0/
> > >
> > > > compatible = "pciclass,0604";
> > > > device_type = "pci";
> > > > #address-cells = <3>;
> > > > #size-cells = <2>;
> > > > ranges;
> > > > bus-range = <0x01 0xff>;
> > > > msi-parent = <&its_pcie>;
> > > >
> > > > pcie@0,0 { // Lane 0: upstream
> > > > reg = <0x010000 0 0 0 0>;
> > >
> > > While technically correct having the bus# in the address, that doesn't
> > > work for FDT since we don't know the bus assignment. So we should just
> > > use 0.
> >
> > Using 0 causes DTB compilation to produce a warning, due to the
> > bus-range.
What's the warning? 'bus-range' should be optional.
> > Without the bus-range, there will be runtime warnings,
> > as this will be assigned as bus 1.
>
> Okay, that might be something we need to fix.
Actually, I don't see how there's a problem. First, the only place we
read 'bus-range' is of_pci_parse_bus_range() and that's only called
for the host bridge. Any other occurrence of 'bus-range' should be
ignored. The only place we read 'reg' is of_pci_get_devfn() and we
mask just the devfn bits.
> [ 4.992572] pci_bus 0000:00: root bus resource [bus 00-01]
I don't see any way this can happen other than pcie@f4000000 node
containing 'bus-range = <0 1>;'. It comes from
pci_host_bridge.windows.
Rob
WARNING: multiple messages have this Message-ID (diff)
From: Rob Herring <robh@kernel.org>
To: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Cc: Linuxarm <linuxarm@huawei.com>,
mauro.chehab@huawei.com,
Binghui Wang <wangbinghui@hisilicon.com>,
Gustavo Pimentel <gustavo.pimentel@synopsys.com>,
Jingoo Han <jingoohan1@gmail.com>,
Xiaowei Song <songxiaowei@hisilicon.com>,
devicetree@vger.kernel.org,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
PCI <linux-pci@vger.kernel.org>,
linux-phy@lists.infradead.org
Subject: Re: [PATCH v3 0/4] DT schema changes for HiKey970 PCIe hardware to work
Date: Tue, 10 Aug 2021 11:52:34 -0600 [thread overview]
Message-ID: <CAL_JsqKr9csV5fPZxD=kRRB5W6RCjHz0VsP6-nx0RQt8mgVJ5w@mail.gmail.com> (raw)
In-Reply-To: <CAL_JsqL-R=kTugNAC-C1gfSm6Xnb0Nw_iLcRki8aQMNQjcLN6A@mail.gmail.com>
On Tue, Aug 10, 2021 at 11:13 AM Rob Herring <robh@kernel.org> wrote:
>
> On Tue, Aug 10, 2021 at 8:21 AM Mauro Carvalho Chehab
> <mchehab+huawei@kernel.org> wrote:
> >
> > Em Tue, 10 Aug 2021 07:44:50 -0600
> > Rob Herring <robh@kernel.org> escreveu:
> >
> > > On Tue, Aug 10, 2021 at 3:42 AM Mauro Carvalho Chehab
> > > <mchehab+huawei@kernel.org> wrote:
> > > >
> > > > Em Fri, 6 Aug 2021 10:23:35 -0600
> > > > Rob Herring <robh@kernel.org> escreveu:
> > > >
> > > > > On Thu, Aug 5, 2021 at 1:58 AM Mauro Carvalho Chehab
> > > > > <mchehab+huawei@kernel.org> wrote:
> > > > > >
> > > > > > Em Thu, 5 Aug 2021 09:46:12 +0200
> > > > > > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:
> > > > > >
> > > > > > > Em Wed, 4 Aug 2021 10:28:53 -0600
> > > > > > > Rob Herring <robh@kernel.org> escreveu:
> > > > > > >
> > > > > > > > On Wed, Aug 04, 2021 at 08:50:45AM +0200, Mauro Carvalho Chehab wrote:
> > > > > > > > > Em Tue, 3 Aug 2021 16:11:42 -0600
> > > > > > > > > Rob Herring <robh+dt@kernel.org> escreveu:
> > > > > > > > >
> > > > > > > > > > On Mon, Aug 2, 2021 at 10:39 PM Mauro Carvalho Chehab
> > > > > > > > > > <mchehab+huawei@kernel.org> wrote:
> > > > > > > > > > >
> > > > > > > > > > > Hi Rob,
> > > > > > > > > > >
> > > > > > > > > > > That's the third version of the DT bindings for Kirin 970 PCIE and its
> > > > > > > > > > > corresponding PHY.
> > > > > > > > > > >
> > > > > > > > > > > It is identical to v2, except by:
> > > > > > > > > > > - pcie@7,0 { // Lane 7: Ethernet
> > > > > > > > > > > + pcie@7,0 { // Lane 6: Ethernet
> > > > > > > > > >
> > > > > > > > > > Can you check whether you have DT node links in sysfs for the PCI
> > > > > > > > > > devices? If you don't, then something is wrong still in the topology
> > > > > > > > > > or the PCI core is failing to set the DT node pointer in struct
> > > > > > > > > > device. Though you don't rely on that currently, we want the topology
> > > > > > > > > > to match. It's possible this never worked on arm/arm64 as mainly
> > > > > > > > > > powerpc relied on this.
> > > > > > > > > >
> > > > > > > > > > I'd like some way to validate the DT matches the PCI topology. We
> > > > > > > > > > could have a tool that generates the DT structure based on the PCI
> > > > > > > > > > topology.
> > > > > > > > >
> > > > > > > > > The of_node node link is on those places:
> > > > > > > > >
> > > > > > > > > $ find /sys/devices/platform/soc/f4000000.pcie/ -name of_node
> > > > > > > > > /sys/devices/platform/soc/f4000000.pcie/of_node
> > > > > > > > > /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/of_node
> > > > > > > > > /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/pci_bus/0000:01/of_node
> > > > > > > > > /sys/devices/platform/soc/f4000000.pcie/pci0000:00/pci_bus/0000:00/of_node
> > > > > > > >
> > > > > > > > Looks like we're missing some...
> > > > > > > >
> > > > > > > > It's not immediately obvious to me what's wrong here. Only the root
> > > > > > > > bus is getting it's DT node set. The relevant code is pci_scan_device(),
> > > > > > > > pci_set_of_node() and pci_set_bus_of_node(). Give me a few days to try
> > > > > > > > to reproduce and debug it.
> > > > > > >
> > > > > > > I added a printk on both pci_set_*of_node() functions:
> > > > > > >
> > > > > > > [ 4.872991] (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
> > > > > > > [ 4.913806] (null): pci_set_of_node: of_node: /soc/pcie@f4000000
> > > > > > > [ 4.978102] pci_bus 0000:01: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > > > > > > [ 4.990622] (null): pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > > > > > > [ 5.052383] pci_bus 0000:02: pci_set_bus_of_node: of_node: (null)
> > > > > > > [ 5.059263] (null): pci_set_of_node: of_node: (null)
> > > > > > > [ 5.085552] (null): pci_set_of_node: of_node: (null)
> > > > > > > [ 5.112073] (null): pci_set_of_node: of_node: (null)
> > > > > > > [ 5.138320] (null): pci_set_of_node: of_node: (null)
> > > > > > > [ 5.164673] (null): pci_set_of_node: of_node: (null)
> > > > > > > [ 5.233759] pci_bus 0000:03: pci_set_bus_of_node: of_node: (null)
> > > > > > > [ 5.240539] (null): pci_set_of_node: of_node: (null)
> > > > > > > [ 5.310545] pci_bus 0000:04: pci_set_bus_of_node: of_node: (null)
> > > > > > > [ 5.324719] pci_bus 0000:05: pci_set_bus_of_node: of_node: (null)
> > > > > > > [ 5.338914] pci_bus 0000:06: pci_set_bus_of_node: of_node: (null)
> > > > > > > [ 5.345516] (null): pci_set_of_node: of_node: (null)
> > > > > > > [ 5.415795] pci_bus 0000:07: pci_set_bus_of_node: of_node: (null)
> > > > > >
> > > > > > The enclosed patch makes the above a clearer:
> > > > > >
> > > > > > [ 4.800975] (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
> > > > > > [ 4.855983] pci 0000:00:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000
> > > > > > [ 4.879169] pci_bus 0000:01: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > > > > > [ 4.900602] pci 0000:01:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > > > > > [ 4.953086] pci_bus 0000:02: pci_set_bus_of_node: of_node: (null)
> > > > >
> > > > > I believe the issue is we need another bridge node in the DT
> > > > > hierarchy. What we have is:
> > > > >
> > > > > Bus 0 is node /soc/pcie@f4000000
> > > > > Bus 1 is device 0 on bus 0 is node /soc/pcie@f4000000/pcie@0,0
> > > > > Bus 2 is device 0 on bus 1 in node ... whoops, there's no device 0
> > > > > under /soc/pcie@f4000000/pcie@0,0
> > > > >
> > > > > So we need the hierarchy to be: /soc/pcie@f4000000/pcie@0/pcie@0/pcie@{1,5,7}
> > > >
> > > > Adding a child pcie@0 produces the following output from my debug
> > > > patches:
> > >
> > > You removed your changes to the PCI code other than the debug print?
> >
> > Yes.
> >
> > > >
> > > > [ 4.984278] (null): pci_set_bus_of_node: of_node: /soc/pcie@f4000000
> > > > [ 5.042992] pci 0000:00:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000
> > > > [ 5.083738] pci_bus 0000:01: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > > > [ 5.124377] pci 0000:01:00.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0
> > > > [ 5.168395] pci_bus 0000:02: pci_set_bus_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> > > > [ 5.200719] pci 0000:02:01.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> > >
> > > This should not happen. The devfn doesn't match.
> > >
> > > > [ 5.247777] pci 0000:02:04.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> > > > [ 5.276768] pci 0000:02:05.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> > > > [ 5.305018] pci 0000:02:07.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> > > > [ 5.333093] pci 0000:02:09.0: pci_set_of_node: of_node: /soc/pcie@f4000000/pcie@0,0/pcie@0,0
> > > > [ 5.395620] pci_bus 0000:03: pci_set_bus_of_node: of_node: (null)
> > > > [ 5.416333] pci 0000:03:00.0: pci_set_of_node: of_node: (null)
> > > > [ 5.451353] pci_bus 0000:04: pci_set_bus_of_node: of_node: (null)
> > > > [ 5.473970] pci_bus 0000:05: pci_set_bus_of_node: of_node: (null)
> > > > [ 5.487765] pci_bus 0000:06: pci_set_bus_of_node: of_node: (null)
> > > > [ 5.530219] pci 0000:06:00.0: pci_set_of_node: of_node: (null)
> > > > [ 5.560896] pci_bus 0000:07: pci_set_bus_of_node: of_node: (null)
> > > >
> > > > It produces the following sysfs nodes:
> > > >
> > > > $ find /sys/devices/platform/soc/f4000000.pcie/ -name of_node
> > > > /sys/devices/platform/soc/f4000000.pcie/of_node
> > > > /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/of_node
> > > > /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/of_node
> > > > /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0/pci_bus/0000:02/of_node
> > > > /sys/devices/platform/soc/f4000000.pcie/pci0000:00/0000:00:00.0/pci_bus/0000:01/of_node
> > > > /sys/devices/platform/soc/f4000000.pcie/pci0000:00/pci_bus/0000:00/of_node
> > > >
> > > >
> > > > I'm enclosing the DT schema I'm using.
> > > >
> > > >
> > > >
> > > > Thanks,
> > > > Mauro
> > > >
> > > > ---
> > > >
> > > > pcie@f4000000 {
> > > > compatible = "hisilicon,kirin970-pcie";
> > > > reg = <0x0 0xf4000000 0x0 0x1000000>,
> > > > <0x0 0xfc180000 0x0 0x1000>,
> > > > <0x0 0xf5000000 0x0 0x2000>;
> > > > reg-names = "dbi", "apb", "config";
> > > > bus-range = <0x00 0xff>;
> > > > #address-cells = <3>;
> > > > #size-cells = <2>;
> > > > device_type = "pci";
> > > > phys = <&pcie_phy>;
> > > > ranges = <0x02000000 0x0 0x00000000
> > > > 0x0 0xf6000000
> > > > 0x0 0x02000000>;
> > > > num-lanes = <1>;
> > > > #interrupt-cells = <1>;
> > > > interrupts = <GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>;
> > > > interrupt-names = "msi";
> > > > interrupt-map-mask = <0 0 0 7>;
> > > > interrupt-map = <0x0 0 0 1
> > > > &gic GIC_SPI 282 IRQ_TYPE_LEVEL_HIGH>,
> > > > <0x0 0 0 2
> > > > &gic GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>,
> > > > <0x0 0 0 3
> > > > &gic GIC_SPI 284 IRQ_TYPE_LEVEL_HIGH>,
> > > > <0x0 0 0 4
> > > > &gic GIC_SPI 285 IRQ_TYPE_LEVEL_HIGH>;
> > > > reset-gpios = <&gpio7 0 0>;
> > > > hisilicon,clken-gpios = <&gpio27 3 0>, <&gpio17 0 0>,
> > > > <&gpio20 6 0>;
> > > > pcie@0,0 { // Lane 0: PCIe switch: Bus 1, Device 0
> > > > reg = <0x80 0 0 0 0>;
> > >
> > > s/0x80/0/
> > >
> > > > compatible = "pciclass,0604";
> > > > device_type = "pci";
> > > > #address-cells = <3>;
> > > > #size-cells = <2>;
> > > > ranges;
> > > > bus-range = <0x01 0xff>;
> > > > msi-parent = <&its_pcie>;
> > > >
> > > > pcie@0,0 { // Lane 0: upstream
> > > > reg = <0x010000 0 0 0 0>;
> > >
> > > While technically correct having the bus# in the address, that doesn't
> > > work for FDT since we don't know the bus assignment. So we should just
> > > use 0.
> >
> > Using 0 causes DTB compilation to produce a warning, due to the
> > bus-range.
What's the warning? 'bus-range' should be optional.
> > Without the bus-range, there will be runtime warnings,
> > as this will be assigned as bus 1.
>
> Okay, that might be something we need to fix.
Actually, I don't see how there's a problem. First, the only place we
read 'bus-range' is of_pci_parse_bus_range() and that's only called
for the host bridge. Any other occurrence of 'bus-range' should be
ignored. The only place we read 'reg' is of_pci_get_devfn() and we
mask just the devfn bits.
> [ 4.992572] pci_bus 0000:00: root bus resource [bus 00-01]
I don't see any way this can happen other than pcie@f4000000 node
containing 'bus-range = <0 1>;'. It comes from
pci_host_bridge.windows.
Rob
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
next prev parent reply other threads:[~2021-08-10 18:09 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-03 4:38 [PATCH v3 0/4] DT schema changes for HiKey970 PCIe hardware to work Mauro Carvalho Chehab
2021-08-03 4:38 ` Mauro Carvalho Chehab
2021-08-03 4:38 ` [PATCH v3 1/4] dt-bindings: PCI: kirin: Fix compatible string Mauro Carvalho Chehab
2021-08-03 22:22 ` Rob Herring
2021-08-03 4:38 ` [PATCH v3 2/4] dt-bindings: PCI: kirin: Convert kirin-pcie.txt to yaml Mauro Carvalho Chehab
2021-08-03 22:27 ` Rob Herring
2021-08-03 4:38 ` [PATCH v3 3/4] dt-bindings: PCI: kirin: Add support for Kirin970 Mauro Carvalho Chehab
2021-08-03 4:38 ` [PATCH v3 4/4] dt-bindings: phy: Add bindings for HiKey 970 PCIe PHY Mauro Carvalho Chehab
2021-08-03 4:38 ` Mauro Carvalho Chehab
2021-08-03 22:29 ` Rob Herring
2021-08-03 22:29 ` Rob Herring
2021-08-03 22:11 ` [PATCH v3 0/4] DT schema changes for HiKey970 PCIe hardware to work Rob Herring
2021-08-03 22:11 ` Rob Herring
2021-08-04 6:50 ` Mauro Carvalho Chehab
2021-08-04 6:50 ` Mauro Carvalho Chehab
2021-08-04 16:28 ` Rob Herring
2021-08-04 16:28 ` Rob Herring
2021-08-05 7:46 ` Mauro Carvalho Chehab
2021-08-05 7:46 ` Mauro Carvalho Chehab
2021-08-05 7:58 ` Mauro Carvalho Chehab
2021-08-05 7:58 ` Mauro Carvalho Chehab
2021-08-06 16:23 ` Rob Herring
2021-08-06 16:23 ` Rob Herring
2021-08-10 9:42 ` Mauro Carvalho Chehab
2021-08-10 9:42 ` Mauro Carvalho Chehab
2021-08-10 13:44 ` Rob Herring
2021-08-10 13:44 ` Rob Herring
2021-08-10 14:20 ` Mauro Carvalho Chehab
2021-08-10 14:20 ` Mauro Carvalho Chehab
2021-08-10 17:13 ` Rob Herring
2021-08-10 17:13 ` Rob Herring
2021-08-10 17:52 ` Rob Herring [this message]
2021-08-10 17:52 ` Rob Herring
2021-08-11 7:11 ` Mauro Carvalho Chehab
2021-08-11 7:11 ` Mauro Carvalho Chehab
2021-08-11 6:46 ` Mauro Carvalho Chehab
2021-08-11 6:46 ` Mauro Carvalho Chehab
2021-08-12 3:13 ` Rob Herring
2021-08-12 3:13 ` Rob Herring
2021-08-12 7:48 ` Mauro Carvalho Chehab
2021-08-12 7:48 ` Mauro Carvalho Chehab
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_JsqKr9csV5fPZxD=kRRB5W6RCjHz0VsP6-nx0RQt8mgVJ5w@mail.gmail.com' \
--to=robh@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=gustavo.pimentel@synopsys.com \
--cc=jingoohan1@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=linux-phy@lists.infradead.org \
--cc=linuxarm@huawei.com \
--cc=mauro.chehab@huawei.com \
--cc=mchehab+huawei@kernel.org \
--cc=songxiaowei@hisilicon.com \
--cc=wangbinghui@hisilicon.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.