Re: [RFC PATCH fwctl 2/5] pds_core: add new fwctl auxilary_device

[Leon Romanovsky <leon@kernel.org>]

On Tue, Feb 18, 2025 at 12:00:52PM -0800, Nelson, Shannon wrote:
> On 2/18/2025 11:28 AM, Leon Romanovsky wrote:
> > 
> > On Tue, Feb 11, 2025 at 03:48:51PM -0800, Shannon Nelson wrote:
> > > Add support for a new fwctl-based auxiliary_device for creating a
> > > channel for fwctl support into the AMD/Pensando DSC.
> > > 
> > > Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
> > > ---
> > >   drivers/net/ethernet/amd/pds_core/auxbus.c |  3 +--
> > >   drivers/net/ethernet/amd/pds_core/core.c   |  7 +++++++
> > >   drivers/net/ethernet/amd/pds_core/core.h   |  1 +
> > >   drivers/net/ethernet/amd/pds_core/main.c   | 10 ++++++++++
> > >   include/linux/pds/pds_common.h             |  2 ++
> > >   5 files changed, 21 insertions(+), 2 deletions(-)
> > 
> > <...>
> > 
> > My comment is only slightly related to the patch itself, but worth to
> > write it anyway.
> > 
> > > diff --git a/include/linux/pds/pds_common.h b/include/linux/pds/pds_common.h
> > > index 5802e1deef24..b193adbe7cc3 100644
> > > --- a/include/linux/pds/pds_common.h
> > > +++ b/include/linux/pds/pds_common.h
> > > @@ -29,6 +29,7 @@ enum pds_core_vif_types {
> > >        PDS_DEV_TYPE_ETH        = 3,
> > >        PDS_DEV_TYPE_RDMA       = 4,
> > >        PDS_DEV_TYPE_LM         = 5,
> > > +     PDS_DEV_TYPE_FWCTL      = 6,
> > 
> > This enum and defines below should be cleaned from unsupported types.
> > I don't see any code for RDMA, LM and ETH.
> > 
> > Thanks
> 
> I've looked at those a few times over the life of this code, but I continue
> to leave them there because they are part of the firmware interface
> definition, whether we use them or not.

How? You are passing some number which FW is not aware of it. You can
pass any number you want there. Even it is not true, you can
PDS_DEV_TYPE_FWCTL = 6 here, but remove rest of enums and *_STR defines.

> 
> You're right, there is no ETH or RDMA type code, they exist as historical
> artifacts of the early interface design.

This make me wonder why netdev merged this code which has nothing to do
with netdev subsystem at all.

Thanks