[PATCH v3 2/2] cmd: brcm: netXtreme commands

[Simon Glass]

Hi Roman,

On Wed, 27 Oct 2021 at 09:05, Roman Bacik <roman.bacik at broadcom.com> wrote:
>
> > -----Original Message-----
> > From: Marek Behún <kabel at kernel.org>
> > Sent: Tuesday, October 26, 2021 9:50 AM
> > To: Roman Bacik <roman.bacik at broadcom.com>
> > Cc: Simon Glass <sjg at chromium.org>; U-Boot Mailing List <u-
> > boot at lists.denx.de>; Bharat Gooty <bharat.gooty at broadcom.com>;
> > Aswath Govindraju <a-govindraju at ti.com>; Bin Meng
> > <bmeng.cn at gmail.com>; Franck LENORMAND
> > <franck.lenormand at nxp.com>; Heinrich Schuchardt
> > <xypron.glpk at gmx.de>; Kory Maincent <kory.maincent at bootlin.com>;
> > Michal Simek <michal.simek at xilinx.com>; Patrick Delaunay
> > <patrick.delaunay at foss.st.com>; Peng Fan <peng.fan at nxp.com>; Priyanka
> > Jain <priyanka.jain at nxp.com>; Rayagonda Kokatanur
> > <rayagonda.kokatanur at broadcom.com>; Sean Anderson
> > <sean.anderson at seco.com>
> > Subject: Re: [PATCH v3 2/2] cmd: brcm: netXtreme commands
> >
> > On Tue, 26 Oct 2021 09:02:54 -0700
> > Roman Bacik <roman.bacik at broadcom.com> wrote:
> >
> > > On Tue, Oct 26, 2021 at 8:55 AM Marek Behún <kabel at kernel.org> wrote:
> > > >
> > > > On Tue, 26 Oct 2021 08:14:28 -0700
> > > > Roman Bacik <roman.bacik at broadcom.com> wrote:
> > > >
> > > > > Hi Marek,
> > > > >
> > > > > We do not want this driver to be automatically probed. It is not
> > > > > needed
> > > > > all the time and also slows down the boot time. We have stripped
> > > > > down
> > > > > everything else to bare minimum.
> > > > > Thanks,
> > > > >
> > > > > Roman
> > > >
> > > > Hi Roman,
> > > >
> > > > OK, that is reasonable, but not reasonable enough to introduce a new
> > > > vendor specific command.
> > > >
> > > > Still NAK.
> > > >
> > > > So you have the bnxt_drv_probe method defined in the driver, but you
> > > > don't set a pointer to it into the U_BOOT_DRIVER structure, and
> > > > instead
> > > > you call this method when "brcm probe" command is called.
> > > >
> > > > I think this introduction of another vendor specific command is wrong.
> > > >
> > > > If probing takes too much time and should be done only when the device
> > > > is needed, there are 2 things you could do:
> > > >
> > > > - you can create new driver flag saying that the device should be
> > > >   probeb only when needed, wire necessary code and add this flag to
> > your
> > > >   driver (this could get very complicated, though)
> > > > - you can do minimum stuff in probe method, and move the stuff that
> > > >   takes long time into bnxt_start(), which is called only when network
> > > >   via this ethernet controller is requested for by U-Boot commands.
> > >
> > > So renaming bnxt probe/remove to bnxt start/stop will do, right?
> >
> > No. The whole idea of adding the new "bnxt" command is wrong, because
> > the command is *vendor specific*. The ethernet controller should work
> > out of the box with standard U-Boot commands, i.e. it if I use the
> >   dhcp
> > command, it should work, without needing to call the "bnxt" command.
>
> Hi Marek,
>
> In order to speed up the boot, we do not load bnxt driver on each boot. Also
> we do not need to load FW and initialize PCI required to bind bnxt. When
> bnxt is required, then we execute these commands:
>
> chimp_ld_secure #this command loads FW, which is necessary for PCIe to
> enumerate it
> pci enum #this command is necessary to call bnxt_bind
> bnxt 0 probe #this command would probe/load the driver
>
> Do you have a suggestion on how to make this work without introducing bnxt
> commands if we do not want to enumerate PCIe, load FW and load bnxt on each
> boot? Currently we boot to uboot prompt in 1s, which is our requirement.
> Thanks,

It sounds like your ethernet driver will not even be bound in the
non-PCI case. Is that right?

What about the PCI driver? Does that get bound initially? If so, I
think U-Boot will normally probe PCI. But we should provide a way to
not auto--probe PCI. Would that help?

You could put the firmware loading (or a call to it) in your PCI
driver, perhaps. Then bringing up Ethernet could be something like:

ret = uclass_first_device_err(UCLASS_PCI, &dev);

I think your use case is not that unusual and we should be able to
support it in a standard way. If there are features missing for that I
am very happy to help figure it out.

Regards,
Simon