[U-Boot] [PATCH] pci: Support parsing PCI controller DT subnodes

Marek Vasut
2018-08-23T07:42:13Z

On 08/23/2018 04:11 AM, Bin Meng wrote:
> Hi Marek,

Hello Bin,

> On Wed, Aug 22, 2018 at 5:57 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> On 08/22/2018 04:14 AM, Bin Meng wrote:
>> [...]
>>>>>> I said in the previous discussion I am willing to update the
>>>>>> documentation to match the implementation, but that's about it.
>>>>>>
>>>>>>> So far almost all PCI device drivers in U-Boot supports both
>>>>>>> scenarios, except PCI UART which currently only supports scenario#1.
>>>>>>> If you declare what you do in this patch is pureblood then you should
>>>>>>> revoke the other 2 at the same time. Leaving such in the mainstream
>>>>>>> only creates chaos in the future and we should avoid doing that, given
>>>>>>> we already had lots of discussion here.
>>>>>>
>>>>>> No. The compatible is valid for some PCI subdevs, ie. bridges, so that
>>>>>> has to stay. You also need compatible for sandbox, you said that
>>>>>> yourself. And declaring a PCI device with BFD in DT is needed, ie. for
>>>>>> the r8a7794-style bindings.
>>>>>>
>>>>>
>>>>> OK, now new comments :) So you admit "compatible" can be valid for
>>>>> some cases.
>>>>
>>>> _some_ select ones, and that is a _very_ important distinction.
>>>>
>>>>> I have to point out that your theory on PCI device probing
>>>>> is self-contradictory.You refuse to add a "compatible" string to your
>>>>> PCI device because you said the vendor/device id/class provides enough
>>>>> information to bind the driver, then why do you want to specify your
>>>>> devices in the device tree in the first place.
>>>>
>>>> Because of the USB PHY , which is attached to that device and can not be
>>>> probed on the PCI bus, unlike the device itself ?
>>>>
>>>
>>> You can create a new EHCI PCI driver to match your USB EHCI
>>> controller's vendor id/device id, and do any additional PHY setup in
>>> that driver. BTW what's the vendor id and device id of the EHCI
>>> controller on your board? I can have a look up at the PCI database.
>>
>> You can not.
>>
>> 1033:00e0 -- https://pci-ids.ucw.cz/read/PC/1033/00e0
>>
>> You can buy that controller as a discrete chip on a PCI card or have one
>> in an SoC on a PCI bus, I have both.
>
> Hardware vendors like to create such fantastic stuff in their SoCs,
> sigh. So how about subvendor and subdevice id? The embedded one can't
> be the same as the the discrete one. If they are really all the same,
> then why can't we add a specific compatible string to describe such
> device? The compatible idea was invented to describe devices that
> cannot be discovered via some probeable ways.

I cannot if you look at the r8a7794 PHY phandles in the DT. The PHY
connects to two devices and this is well modeled in DT.

>>>>> According to your
>>>>> theory, "Each PCI device already has a PCI ID and class which is used
>>>>> to identify it and based on which the drivers bind to it, so a DT
>>>>> compatible is NOT needed and is actually redundant and harmful.", I
>>>>> would argue why not just creating a dedicate PCI device driver using
>>>>> PCI ID and class information to do the driver binding and start from
>>>>> there.
>>>>
>>>> Because the same device with the same PCI ID can be used without that PHY .
>>>>
>>>>> As you mentioned PCI bus is probable bus like USB, everything
>>>>> can be self-contained in the PCI device driver itself. There is no
>>>>> need to create nothing in the device tree. If you want an example,
>>>>> check 8250_pci.c in the Linux kernel. Everything that is needed to
>>>>> configure the driver is included in the driver itself. It does not
>>>>> read anything from the device tree.
>>>>
>>>> Well, see above for why this approach doesn't work.
>>>
>>> Please explain why it does not work. I see Intel e1000 driver in the
>>> Linux kernel has lots of PHY configuration within the driver. It does
>>> not use any external PHY (eg: Marvell 88exxx) driver that Linux
>>> already has. The Ethernet PHY are not probeable on the PCI bus too.
>>
>> That's a different thing, the ethernet MACs have an internal MDIO
>> interface and you can scan the MDIO bus and read out PHY registers 0/1
>> to get the PHY ID.
>>
>
> Yes, I know. But my point is that Linux drivers are not always
> consistent. There is diversity and Linux is fine with that. There is
> also diversity when it comes to U-Boot PCI support and we have 2
> supported PCI scenarios, the 1st of which can satisfy your use case.
>
>> This does not apply to this particular usecase here. In this case, the
>> controller can be either embedded or discrete, with PHY or without PHY.
>>
>
> Does the embedded one have no register that can identify the PHY presence?

No, why would it ? They are two completely separate blocks .

[...]

--
Best regards,
Marek Vasut