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

Simon Glass
2016-01-22T21:50:45Z

Hi Stephen,

On 22 January 2016 at 14:48, Stephen Warren <swarren@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@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@gmail.com> wrote:
>>>>>
>>>>>
>>>>> On Fri, Jan 22, 2016 at 11:36 AM, Simon Glass <sjg@chromium.org> wrote:
>>>>>>
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> On 21 January 2016 at 18:39, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>>>>>
>>>>>>>
>>>>>>> Hi Stephen,
>>>>>>>
>>>>>>> On Fri, Jan 22, 2016 at 7:35 AM, Stephen Warren
>>>>>>> <swarren@wwwdotorg.org> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> From: Stephen Warren <swarren@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@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.

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