public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Stephen Warren <swarren@wwwdotorg.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] pci: restore initialization for DM_PCI
Date: Fri, 22 Jan 2016 14:58:06 -0700	[thread overview]
Message-ID: <56A2A5EE.8080005@wwwdotorg.org> (raw)
In-Reply-To: <CAPnjgZ29UVLjunk1Kho=z=+PX+rDWQAGUNtOGMcHr7WwCYRQUA@mail.gmail.com>

On 01/22/2016 02:50 PM, Simon Glass wrote:
> 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.

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.

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.

  reply	other threads:[~2016-01-22 21:58 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-21 23:35 [U-Boot] [PATCH] pci: restore initialization for DM_PCI Stephen Warren
2016-01-22  0:30 ` Tom Rini
2016-01-22  1:39 ` Bin Meng
2016-01-22  3:36   ` Simon Glass
2016-01-22  3:53     ` Bin Meng
2016-01-22  4:03       ` Simon Glass
2016-01-22  4:06         ` Bin Meng
2016-01-22  4:17           ` Simon Glass
2016-01-22 19:38         ` Stephen Warren
2016-01-22 20:32           ` Simon Glass
2016-01-22 21:48             ` Stephen Warren
2016-01-22 21:50               ` Simon Glass
2016-01-22 21:58                 ` Stephen Warren [this message]
2016-01-22 22:00                   ` Simon Glass
2016-01-25 17:11                     ` Stephen Warren
2016-01-26  0:57                       ` Simon Glass
2016-01-24 12:23               ` Bin Meng
2016-01-22 19:35   ` Stephen Warren
2016-01-22 15:49 ` Thierry Reding

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56A2A5EE.8080005@wwwdotorg.org \
    --to=swarren@wwwdotorg.org \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox