Re: [RFC PATCH fwctl 3/5] pds_fwctl: initial driver framework

[Leon Romanovsky <leon@kernel.org>]

On Thu, Feb 20, 2025 at 03:27:37PM -0800, Nelson, Shannon wrote:
> On 2/19/2025 12:25 AM, Leon Romanovsky wrote:
> > 
> > On Tue, Feb 18, 2025 at 02:19:03PM -0800, Nelson, Shannon wrote:
> > > On 2/18/2025 11:51 AM, Leon Romanovsky wrote:
> > > > 
> > > > On Tue, Feb 11, 2025 at 03:48:52PM -0800, Shannon Nelson wrote:
> > > > > Initial files for adding a new fwctl driver for the AMD/Pensando PDS
> > > > > devices.  This sets up a simple auxiliary_bus driver that registers
> > > > > with fwctl subsystem.  It expects that a pds_core device has set up
> > > > > the auxiliary_device pds_core.fwctl
> > > > > 
> > > > > Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
> > > > > ---
> > > > >    MAINTAINERS                    |   7 ++
> > > > >    drivers/fwctl/Kconfig          |  10 ++
> > > > >    drivers/fwctl/Makefile         |   1 +
> > > > >    drivers/fwctl/pds/Makefile     |   4 +
> > > > >    drivers/fwctl/pds/main.c       | 195 +++++++++++++++++++++++++++++++++
> > > > >    include/linux/pds/pds_adminq.h |  77 +++++++++++++
> > > > >    include/uapi/fwctl/fwctl.h     |   1 +
> > > > >    include/uapi/fwctl/pds.h       |  27 +++++
> > > > >    8 files changed, 322 insertions(+)
> > > > >    create mode 100644 drivers/fwctl/pds/Makefile
> > > > >    create mode 100644 drivers/fwctl/pds/main.c
> > > > >    create mode 100644 include/uapi/fwctl/pds.h
> > > > 
> > > > <...>
> > 
> > <...>
> > 
> > > > > +             return err;
> > > > > +     }
> > > > > +
> > > > > +     cmd.fwctl_ident.opcode = PDS_FWCTL_CMD_IDENT;
> > > > > +     cmd.fwctl_ident.version = 0;
> > > > 
> > > > How will you manage this version field?
> > > 
> > > Since there is only version 0 at this point, there is not much to manage.  I
> > > wanted to explicitly show the setting to version 0, but maybe that can be
> > > assumed by the basic struct init.
> > 
> > But the question is slightly different "How will you manage this version field?"
> 
> If we find we have to change the interface in a non-backward-compatable way,
> we'll increment the version number that we support, and watch for the
> version number supported by the firmware as reported in the ident struct
> data and interpret the data appropriately.  Similarly, if the firmware sees
> that the host driver is at a lower version number, it will handle data in
> the older format.

Thanks.

> 
> sln
> 
> 
>