[U-Boot] [PATCH] pci: restore initialization for DM_PCI

[Simon Glass]

Hi Stephen,

On 22 January 2016 at 14:58, Stephen Warren <swarren at wwwdotorg.org> wrote:
> On 01/22/2016 02:50 PM, Simon Glass wrote:
>>
>> Hi Stephen,
>>
>> On 22 January 2016 at 14:48, Stephen Warren <swarren at wwwdotorg.org> wrote:
>>>
>>> On 01/22/2016 01:32 PM, Simon Glass wrote:
>>>>
>>>>
>>>> Hi Stephen,
>>>>
>>>> On 22 January 2016 at 12:38, Stephen Warren <swarren at wwwdotorg.org>
>>>> wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 01/21/2016 09:03 PM, Simon Glass wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> Hi Bin,
>>>>>>
>>>>>> On 21 January 2016 at 20:53, Bin Meng <bmeng.cn at gmail.com> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Fri, Jan 22, 2016 at 11:36 AM, Simon Glass <sjg at chromium.org>
>>>>>>> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> On 21 January 2016 at 18:39, Bin Meng <bmeng.cn at gmail.com> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Hi Stephen,
>>>>>>>>>
>>>>>>>>> On Fri, Jan 22, 2016 at 7:35 AM, Stephen Warren
>>>>>>>>> <swarren at wwwdotorg.org> wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> From: Stephen Warren <swarren at nvidia.com>
>>>>>>>>>>
>>>>>>>>>> PCI controllers should be enumerated at startup so that PCI
>>>>>>>>>> devices
>>>>>>>>>> such as Ethernet controllers are available at startup. Fix
>>>>>>>>>> board_init_r()
>>>>>>>>>> not to skip calling pci_init() when CONFIG_DM_PCI is defined, and
>>>>>>>>>> provide
>>>>>>>>>> an implementation of pci_init() for the DM case.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> What exact issue are you trying to fix? I posted the same question
>>>>>>>>> on
>>>>>>>>> Simon's patch [1] before. Does your patch and Simon's fix the same
>>>>>>>>> issue?
>>>>>>>>>
>>>>>>>>> Note I submitted a similar patch [2] last year for x86 only, to
>>>>>>>>> explicitly trigger the PCI enueration. But it was not accepted.
>>>>>>>>>
>>>>>>>>>> Fixes: 96350f729c42 ("dm: tegra: net: Convert tegra boards to
>>>>>>>>>> driver
>>>>>>>>>> model
>>>>>>>>>> for Ethernet")
>>>>>>>>>> Signed-off-by: Stephen Warren <swarren at nvidia.com>
>>>>>>>>>> ---
>>>>>>>>>> I'm not sure if relying on the side-effects of calling
>>>>>>>>>> uclass_{first,ext}_device is the correct approach; is there a more
>>>>>>>>>> explicit
>>>>>>>>>> way to probe all PCI controllers?
>>>>>>>>>>
>>>>>>>>>> Arguably, perhaps we should introduce a "pci start" command
>>>>>>>>>> instead
>>>>>>>>>> of
>>>>>>>>>> this change to be consistent with e.g. USB. However, that would be
>>>>>>>>>> a
>>>>>>>>>> regression relative to earlier versions of U-Boot.
>>>>>>>>>> ---
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> [1] http://patchwork.ozlabs.org/patch/569323/
>>>>>>>>> [2] http://patchwork.ozlabs.org/patch/500246/
>>>>>>>>>
>>>>>>>>> Regards,
>>>>>>>>> Bin
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> This does go against the driver-model philosophy of lazy init. I
>>>>>>>> wonder if we should add this patch with a Kconfig option to enable
>>>>>>>> it?
>>>>>>>> Then it can be enabled only for boards that need it.
>>>>>>>>
>>>>>>>
>>>>>>> I suspect the issue is somewhere else. On Intel Galileo with a PCI
>>>>>>> ethernet, it works fine without such explicit pci init. Which PCI
>>>>>>> ethernet driver does not work on Tegra?
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> It could be because that board probes PCI to get its serial to work.
>>>>>>
>>>>>> This could be fixed on Tegra by adding an Ethernet node to the device
>>>>>> tree to cause it to be probed. But I don't think that should be a
>>>>>> requirement.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Since PCI devices are automatically probed via standard bus protocols,
>>>>> I
>>>>> believe we shouldn't have to add them to the DT.
>>>>>
>>>>> However, I could accept that we should add the PCI controller to DT in
>>>>> order for the controller to be probed, and when that happens, the bus
>>>>> should
>>>>> be enumerated thus causing all the Ethernet devices to be probed.
>>>>> However,
>>>>> the PCI controller is already in DT, and this process isn't being
>>>>> kicked
>>>>> off, so if that's the way it's supposed to work, there's a bug there.
>>>>
>>>>
>>>>
>>>> So what do you think about a Kconfig option that tells U-Boot that PCI
>>>> must be probed after relocation?
>>>
>>>
>>>
>>> I'm puzzled why anyone would turn off that option.
>>>
>>> If PCI is to be used at all, it needs to be probed. The only way I'm
>>> aware
>>> of that it can be probed today is by accidental side-effect of some board
>>> forcibly probing some device that happens to be on a PCI bus (i.e. Bin's
>>> case). In other case, PCI doesn't get automatically probed at boot, and I
>>> don't see any command that allows it to be probed manually. Don't we need
>>> this patch (or your patch linked above) in order to make PCI usable at
>>> all?
>>> If so, why would anyone turn this off?
>>
>>
>> We try to probe devices only when used. E.g. on beaver, if you are not
>> net-booting, you actually don't need PCI.
>
>
> Are you saying that the first network command I execute in U-Boot should
> probe Ethernet device and hence PCI devices right now in the current U-Boot
> code?
>
> That doesn't work right now. If it did work, that would probably be OK.

Two options:
- Add a 'pci start' command to your script
- Enable the PCI auto-probe Kconfig option

Boards can then choose which they prefer.

>
> If not, I don't see how mentioning the lazy probing is relevant.
>
>
>> I think a command is a good idea. I also think a Kconfig option to
>> auto-probe PCI is worth adding. But I'm not keen on manually probing
>> it by default for all PCI boards.

Regards,
Simon