Re: [dpdk-dev] [PATCH v3 1/2] net/ice: factorize firmware loading

["Wang, Haiyue" <haiyue.wang@intel.com>]

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Monday, July 5, 2021 15:08
> To: Wang, Haiyue <haiyue.wang@intel.com>
> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v3 1/2] net/ice: factorize firmware loading
> 
> On Mon, Jul 5, 2021 at 3:43 AM Wang, Haiyue <haiyue.wang@intel.com> wrote:
> >
> > Hi David,
> >
> > > -----Original Message-----
> > > From: dev <dev-bounces@dpdk.org> On Behalf Of David Marchand
> > > Sent: Tuesday, June 29, 2021 16:07
> > > To: dev@dpdk.org
> > > Cc: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> > > Subject: [dpdk-dev] [PATCH v3 1/2] net/ice: factorize firmware loading
> > >
> > > Both "normal" and "dcf" inits have their copy of some firmware loading
> > > code.
> > >
> > > The DSN query is moved in specific parts for the "normal" and "dcf" init.
> > >
> > > A common helper ice_load_pkg is then introduced and takes an adapter
> > > pointer as its main input.
> > >
> > > This helper takes care of finding the right firmware file and loading
> > > it.
> > > The adapter active_pkg_type field is set by this helper.
> > >
> > > The ice_access macro is removed from the osdep.h header: osdep.h should
> > > only hosts wrappers for base driver code.
> > >
> > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > ---
> > >  drivers/net/ice/base/ice_osdep.h |   6 --
> > >  drivers/net/ice/ice_dcf_parent.c |  97 ++-----------------
> > >  drivers/net/ice/ice_ethdev.c     | 161 +++++++++++++++----------------
> > >  drivers/net/ice/ice_ethdev.h     |   3 +-
> > >  4 files changed, 88 insertions(+), 179 deletions(-)
> > >
> >
> >
> > > +     if (!use_dsn)
> > > +             goto no_dsn;
> > > +
> > > +     memset(opt_ddp_filename, 0, ICE_MAX_PKG_FILENAME_SIZE);
> > > +     snprintf(opt_ddp_filename, ICE_MAX_PKG_FILENAME_SIZE,
> > > +             "ice-%016" PRIx64 ".pkg", dsn);
> > > +     strncpy(pkg_file, ICE_PKG_FILE_SEARCH_PATH_UPDATES,
> > > +             ICE_MAX_PKG_FILENAME_SIZE);
> > > +     if (!ice_access(strcat(pkg_file, opt_ddp_filename), 0))
> > > +             goto load_fw;
> > > +
> > > +     strncpy(pkg_file, ICE_PKG_FILE_SEARCH_PATH_DEFAULT,
> > > +             ICE_MAX_PKG_FILENAME_SIZE);
> > > +     if (!ice_access(strcat(pkg_file, opt_ddp_filename), 0))
> > > +             goto load_fw;
> > > +
> > > +no_dsn:
> > > +     strncpy(pkg_file, ICE_PKG_FILE_UPDATES, ICE_MAX_PKG_FILENAME_SIZE);
> > > +     if (!ice_access(pkg_file, 0))
> > > +             goto load_fw;
> > > +     strncpy(pkg_file, ICE_PKG_FILE_DEFAULT, ICE_MAX_PKG_FILENAME_SIZE);
> > > +     if (ice_access(pkg_file, 0)) {
> > >               PMD_INIT_LOG(ERR, "failed to search file path\n");
> > > -             return err;
> > > +             return -1;
> > >       }
> > >
> > > +load_fw:
> > >       file = fopen(pkg_file, "rb");
> > >       if (!file)  {
> > >               PMD_INIT_LOG(ERR, "failed to open file: %s\n", pkg_file);
> > >               return -1;
> > >       }
> > >
> >
> > I'm wondering what's full name for ice firmware in F34, has any *.xz
> > postfix ? If so, the search method will also needs to be updated, since
> > we will check each file can be accessed:
> >
> > #define ICE_PKG_FILE_DEFAULT "/lib/firmware/intel/ice/ddp/ice.pkg"
> > #define ICE_PKG_FILE_UPDATES "/lib/firmware/updates/intel/ice/ddp/ice.pkg"
> 
> This first patch is a preparation to have a single helper to
> select/open the firmware.
> I don't get what you mean.

Since the pkg file has the *.xz, now the search method doesn't work. You fix
the read only. ;-)

> 
> Is there a change in behavior with this patch?
> 
> 
> --
> David Marchand