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

[Roman Bacik]

On Tue, Oct 26, 2021 at 9:52 AM Marek Behún <kabel at kernel.org> wrote:
>
> On Tue, 26 Oct 2021 09:40:44 -0700
> Roman Bacik <roman.bacik at broadcom.com> wrote:
>
> > On Tue, Oct 26, 2021 at 9:02 AM 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?
> > >
> > > >
> > > > Also, you're still doing
> > > >
> > > > +       if (env_get("ethaddr"))
> > > > +               secondary = 1;
> > >
> > > Why can't we access the env variable from our "bnxt start" method? Is
> > > there a blacklist of env variables one must not access from a driver?
> >
> > Marek,
> >
> > Sometimes we can have two ethernet devices. One is 10/100/1000M rgmii and
> > another is chip internal 10/100G bnxt. If rgmii is there as eth0, we
> > are incrementing eth number for bnxt:
> >
> > if (env_get("ethaddr"))
> >     secondary = 1;
> > eth_env_get_enetaddr_by_index("eth",bp->cardnum+secondary,bp->mac_set);
> >
> > This way the driver can find that rmii has already taken eth0 so it
> > will use eth1 instead. Do you have a suggestion to work around this?
> > Thanks,
>
> I just replied to your first reply:
>
> Every ethernet controller should use a specific ethNaddr, where
> N = dev_seq(dev) of that controller (and if N=0, it is omitted
> entirely).
>
> So no magical things such as
>   bp->cardnum+secondary
>
> instead you use dev_seq(dev), which gives you the correct number, i.e.
> for N-th UCLASS_ETH device bound it return the number N.
>
> Marek

Marek,

Thank you very much for your suggestion, we will try it.

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.