Hi Marek, On Wed, Sep 19, 2018 at 9:28 PM Marek Vasut <marek.va...@gmail.com> wrote: > > On 09/19/2018 11:41 AM, Bin Meng wrote: > > Hi Marek, > > > > On Wed, Sep 19, 2018 at 5:34 PM Marek Vasut <marek.va...@gmail.com> wrote: > >> > >> On 09/19/2018 11:26 AM, Bin Meng wrote: > >>> Hi Marek, > >>> > >>> On Wed, Sep 19, 2018 at 4:21 PM Marek Vasut <marek.va...@gmail.com> wrote: > >>>> > >>>> On 09/18/2018 03:52 PM, Simon Glass wrote: > >>>>> Hi Marek, > >>>>> > >>>>> On 18 September 2018 at 13:36, Marek Vasut <marek.va...@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.va...@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.va...@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.va...@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.va...@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.va...@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+rene...@gmail.com> > >>>>>>>>>>>>>>>>> Cc: Simon Glass <s...@chromium.org> > >>>>>>>>>>>>>>>>> Cc: Tom Rini <tr...@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 driver binds with a compatible and then pins the read/write config > >>>>>> reg accessors to emulate their return values. Those include PCI IDs. So > >>>>>> you cannot instantiate virtual PCI device without this compatible > >>>>>> string > >>>>>> and thus also cannot write such a test easily. > >>>>>> > >>>>>> Now I also understand where this whole discussion about compatible > >>>>>> strings came from though. > >>>>> > >>>>> The compatible string is needed for the emulation driver but not for > >>>>> the thing that connects to it. However as things stand you can't > >>>>> attach an emulator to a bus without nesting it under the device which > >>>>> it attaches to. > >>>>> > >>>>> I suspect the best answer is to move the emulator so it is a direct > >>>>> child of the bus. You would need to update sandbox_pci_get_emul() to > >>>>> call device_find_first_child() on 'bus' instead of 'dev'. > >>>> > >>>> Sounds to me _way_ out of scope for this patchset. > >>> > >>> Dynamic binding is already supported on Sandbox. I guess Simon may > >>> have missed the part. > >> > >> Well, where is an example of that ? Because I am not seeing one. > >> > > > > I already pointed out in the previous email. In > > arch/sandbox/dts/test.dts, the 2nd PCI controller has two swap_case > > devices and the 3rd controller has one. > > By "second" you mean pci1: or pci2: ? Because pci1: is second , after > pci0 . It'd really help if you were clearer in what you refer to. >
It's pci1. You can see there is no subnode under pci1 there yet if you type 'pci 1' from the U-Boot shell you see two PCI devices. > > In swap_case.c, U_BOOT_PCI_DEVICE() is there which is also a clear > > sign that the driver supports dynamic binding. Of course, the driver > > supports "compatible" too as you noticed. > > Are you talking about sandbox,dev-info DT property here ? This is the property Sandbox uses to make the dynamic binding work. You can bypass this. The key here is that swap_case driver supports both "compatible" and dynamic binding, so you can write test cases to cover this newly added ofnode scenario. Regards, Bin _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot