[U-Boot] [PATCH V2 1/2] pci: Support parsing PCI controller DT subnodes

[Bin Meng]

Hi Marek,

On Wed, Sep 19, 2018 at 4:19 PM Marek Vasut <marek.vasut at gmail.com> wrote:
>
> On 09/18/2018 03:52 PM, Bin Meng wrote:
> > Hi Marek,
> >
> > On Tue, Sep 18, 2018 at 8:01 PM Marek Vasut <marek.vasut at gmail.com> wrote:
> >>
> >> On 09/14/2018 06:41 AM, Simon Glass wrote:
> >>> Hi Marek,
> >>>
> >>> On 10 September 2018 at 01:38, Marek Vasut <marek.vasut at gmail.com> wrote:
> >>>>
> >>>> On 09/02/2018 03:07 AM, Simon Glass wrote:
> >>>>> Hi Marek,
> >>>>
> >>>> Hi,
> >>>>
> >>>>> On 1 September 2018 at 16:45, Marek Vasut <marek.vasut at gmail.com> wrote:
> >>>>>>
> >>>>>> On 09/01/2018 11:50 PM, Simon Glass wrote:
> >>>>>>> Hi Marek,
> >>>>>>>
> >>>>>>> On 30 August 2018 at 07:42, Marek Vasut <marek.vasut at gmail.com> wrote:
> >>>>>>>> On 08/30/2018 03:32 PM, Bin Meng wrote:
> >>>>>>>>> Hi Marek,
> >>>>>>>>>
> >>>>>>>>> On Thu, Aug 30, 2018 at 1:07 AM Marek Vasut <marek.vasut at gmail.com> wrote:
> >>>>>>>>>>
> >>>>>>>>>> On 08/29/2018 05:15 PM, Bin Meng wrote:
> >>>>>>>>>>> +Simon
> >>>>>>>>>>>
> >>>>>>>>>>> Hi Marek,
> >>>>>>>>>>>
> >>>>>>>>>>> On Wed, Aug 29, 2018 at 10:22 PM Marek Vasut <marek.vasut at gmail.com> wrote:
> >>>>>>>>>>>>
> >>>>>>>>>>>> On 08/24/2018 08:27 PM, Marek Vasut wrote:
> >>>>>>>>>>>>> The PCI controller can have DT subnodes describing extra properties
> >>>>>>>>>>>>> of particular PCI devices, ie. a PHY attached to an EHCI controller
> >>>>>>>>>>>>> on a PCI bus. This patch parses those DT subnodes and assigns a node
> >>>>>>>>>>>>> to the PCI device instance, so that the driver can extract details
> >>>>>>>>>>>>> from that node and ie. configure the PHY using the PHY subsystem.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> 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>
> >>>>>>>>>>>>
> >>>>>>>>>>>> Well, bump ?
> >>>>>>>>>>>>
> >>>>>>>>>>>> This is the only missing patch to get my hardware working properly.
> >>>>>>>>>>>
> >>>>>>>>>>> I don't think we ever had an agreement on the v1 patch. Simon had a
> >>>>>>>>>>> long email that pointed out what Linux does seems like a 'fallback' to
> >>>>>>>>>>> find a node with no compatible string.
> >>>>>>>>>>>
> >>>>>>>>>>> Back to this, if we have to go with this way, please create a test
> >>>>>>>>>>> case to cover this scenario.
> >>>>>>>>>>
> >>>>>>>>>> The fact that it works on a particular board is not tested enough?
> >>>>>>>>>> Do we need a custom, special, synthetic test ?
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> I believe that's always been the requirement against the DM code
> >>>>>>>>> changes. I was requested in the past when I changed something in the
> >>>>>>>>> DM and I see other people were asked to do so. Like Alex said, it does
> >>>>>>>>> not mean this patch was not tested enough, but to ensure future
> >>>>>>>>> commits won't break this.
> >>>>>>>>
> >>>>>>>> So, do you have any suggestion how to implement this test ? It seems
> >>>>>>>> Alex posed the same question. It doesn't seem to be trivial in the
> >>>>>>>> context of sandbox.
> >>>>>>>
> >>>>>>> I suppose you need a PCI_DEVICE() declaration for sandbox, with an
> >>>>>>> associated DT node and no compatible string. Then check that you can
> >>>>>>> locate the device and that it read a DT property correctly.
> >>>>>>
> >>>>>> Is there any example of this stuff already ?
> >>>>>
> >>>>> See the bottom of swap_case.c. You might be able to add a new one of those,
> >>>>>
> >>>>> If you look at pci-controller2 in test.dts it has a device with a
> >>>>> compatible string. You could try adding a second device with no
> >>>>> compatible string.
> >>>>
> >>>> And how does that test anything ?
> >>>
> >>> You can test that your code actually attaches the DT node to the
> >>> probed device. Without you code the test would fail. Wit it, it would
> >>> pass.
> >>
> >> Well it won't, because the sandbox swap_case.c requires the compatible.
> >> This all seems like a big hack to support virtual PCI devices.
> >>
> >
> > The sandbox swap_case.c indeed supports dynamic driver binding, just
> > like real PCI devices. Please check doc/driver-model/pci-info.txt
> > (since you were modifying the same doc, I guess you missed that part
> > ..)
>
> Any specific part I am looking for ?

In the pci-info.txt, search for "The sandbox PCI drivers also support
dynamic driver binding". The arch/sandbox/dts/test.dts already has one
PCI controller and two swap_case devices setup for this testing. You
can start from there.

Regards,
Bin