[U-Boot] [PATCH 5/6] test: Add PCI device entry without compat string and with DT node

[Bin Meng]

Hi Marek,

On Mon, Oct 8, 2018 at 12:00 AM Marek Vasut <marek.vasut at gmail.com> wrote:
>
> On 10/07/2018 02:16 PM, Bin Meng wrote:
> > Hi Marek,
> >
> > On Sun, Oct 7, 2018 at 8:12 PM Marek Vasut <marek.vasut at gmail.com> wrote:
> >>
> >> On 10/01/2018 01:44 PM, Marek Vasut wrote:
> >>> On 09/25/2018 05:26 PM, Bin Meng wrote:
> >>>> Hi Marek,
> >>>>
> >>>> On Sat, Sep 22, 2018 at 7:02 AM Marek Vasut <marek.vasut at gmail.com> wrote:
> >>>>>
> >>>>> Add PCI entry without compatible string and with a DT node only with
> >>>>> reg = <...> property into the DT. This is needed for the tests to
> >>>>> verify whether such a setup creates an U-Boot PCI device with the
> >>>>> DT node associated with it in udevice.node.
> >>>>>
> >>>>> Signed-off-by: Marek Vasut <marek.vasut+renesas at gmail.com>
> >>>>> Cc: Simon Glass <sjg at chromium.org>
> >>>>> Cc: Tom Rini <trini at konsulko.com>
> >>>>> ---
> >>>>>  arch/sandbox/dts/test.dts | 9 +++++++--
> >>>>>  1 file changed, 7 insertions(+), 2 deletions(-)
> >>>>>
> >>>>> diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> >>>>> index b8524e3b7d..c13a270c2e 100644
> >>>>> --- a/arch/sandbox/dts/test.dts
> >>>>> +++ b/arch/sandbox/dts/test.dts
> >>>>> @@ -354,9 +354,14 @@
> >>>>>                 #address-cells = <3>;
> >>>>>                 #size-cells = <2>;
> >>>>>                 ranges = <0x02000000 0 0x30000000 0x30000000 0 0x2000
> >>>>> -                               0x01000000 0 0x40000000 0x40000000 0 0x2000>;
> >>>>> +                         0x01000000 0 0x40000000 0x40000000 0 0x2000
> >>>>> +                         0x00008000 0 0x00000000 0x00008000 0 0x2000>;
> >>>>
> >>>> Adding this line makes no sense. You can't translate a PCI bus
> >>>> configuration space address (0x8000) to something in its parent bus's
> >>>> (MMIO) address space. See my related comments in patch 1 and 3.
> >>>>
> >>>
> >>> So what should be in that line ?
> >>
> >> Ping ?
> >
> > Looks you missed my response ?
> > https://lists.denx.de/pipermail/u-boot/2018-October/342820.html
>
> I don't have that mail, sorry.
>
> Can you explain your reply ? Why "There is no need to add that line in
> the ranges property." ?

I mentioned "See my related comments in patch 1 and 3.", and in the
patch 3 review comments [1] I said:

"Using API ofnode_get_addr_size() is wrong. It cannot handle
PCI-specific address formats. I understand why you added "0x00008000 0
0x00000000 0x00008000 0 0x2000" to the bus ranges property in patch 5,
is to make ofnode_get_addr_size() work, but that's the wrong approach.
The correct API should be ofnode_read_pci_addr(). To call it like
this:

ret = ofnode_read_pci_addr(node, FDT_PCI_SPACE_CONFIG, "reg", &addr);
if (!ret)
    df = addr.phys_hi & 0xff00;"

So all we need do is to use ofnode_read_pci_addr() to get the PCI bdf,
and there is no need to add any line in the ranges property.

BTW: changing ranges property also violates the rules of using Linux
DT out of the box :)

[1] https://lists.denx.de/pipermail/u-boot/2018-September/341814.html

Regards,
Bin