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

Wed Oct 27 18:05:09 CEST 2021

Hi Simon,

> -----Original Message-----
> From: Simon Glass <sjg at chromium.org>
> Sent: Wednesday, October 27, 2021 8:41 AM
> To: Roman Bacik <roman.bacik at broadcom.com>
> Cc: Marek Behún <kabel at kernel.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
>
> 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?

Yes correct

>
> 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?

PCI is probed automatically but PCI must be enumerated only after FW is
loaded, which we do not want to be executed each boot.

>
> 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);

Where would we call this from? Will it include calling "pci enum" required
for bnxt to bind? We need this sequence somewhere:

load fw -> pci enum (calls bnxt_bind which sets device name) -> bnxp_probe
(needs bound device by name)

>
> 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

Maybe probing on demand when more than one eth driver can be used could be
acceptable?
Thank you very much for your help,

Roman

-- 
This electronic communication and the information and any files transmitted 
with it, or attached to it, are confidential and are intended solely for 
the use of the individual or entity to whom it is addressed and may contain 
information that is confidential, legally privileged, protected by privacy 
laws, or otherwise restricted from disclosure to anyone else. If you are 
not the intended recipient or the person responsible for delivering the 
e-mail to the intended recipient, you are hereby notified that any use, 
copying, distributing, dissemination, forwarding, printing, or copying of 
this e-mail is strictly prohibited. If you received this e-mail in error, 
please return the e-mail to the sender, delete it from your computer, and 
destroy any printed copy of it.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4206 bytes
Desc: S/MIME Cryptographic Signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20211027/492b2485/attachment.bin>