diff mbox series

[iwl-next,v2,05/15] ice: allocate devlink for subfunction

Message ID 20240513083735.54791-6-michal.swiatkowski@linux.intel.com
State Changes Requested
Headers show
Series ice: support devlink subfunction | expand

Commit Message

Michal Swiatkowski May 13, 2024, 8:37 a.m. UTC
From: Piotr Raczynski <piotr.raczynski@intel.com>

Make devlink allocation function generic to use it for PF and for SF.

Add function for SF devlink port creation. It will be used in next
patch.

Create header file for subfunction device. Define subfunction device
structure there as it is needed for devlink allocation and port
creation.

Signed-off-by: Piotr Raczynski <piotr.raczynski@intel.com>
Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
---
 .../net/ethernet/intel/ice/devlink/devlink.c  | 33 ++++++++++++
 .../net/ethernet/intel/ice/devlink/devlink.h  |  1 +
 .../ethernet/intel/ice/devlink/devlink_port.c | 50 +++++++++++++++++++
 .../ethernet/intel/ice/devlink/devlink_port.h |  3 ++
 drivers/net/ethernet/intel/ice/ice_sf_eth.h   | 21 ++++++++
 5 files changed, 108 insertions(+)
 create mode 100644 drivers/net/ethernet/intel/ice/ice_sf_eth.h

Comments

Kalesh Anakkur Purayil May 13, 2024, 9:25 a.m. UTC | #1
On Mon, May 13, 2024 at 2:03 PM Michal Swiatkowski
<michal.swiatkowski@linux.intel.com> wrote:
>
> From: Piotr Raczynski <piotr.raczynski@intel.com>
>
> Make devlink allocation function generic to use it for PF and for SF.
>
> Add function for SF devlink port creation. It will be used in next
> patch.
>
> Create header file for subfunction device. Define subfunction device
> structure there as it is needed for devlink allocation and port
> creation.
>
> Signed-off-by: Piotr Raczynski <piotr.raczynski@intel.com>
> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> ---
>  .../net/ethernet/intel/ice/devlink/devlink.c  | 33 ++++++++++++
>  .../net/ethernet/intel/ice/devlink/devlink.h  |  1 +
>  .../ethernet/intel/ice/devlink/devlink_port.c | 50 +++++++++++++++++++
>  .../ethernet/intel/ice/devlink/devlink_port.h |  3 ++
>  drivers/net/ethernet/intel/ice/ice_sf_eth.h   | 21 ++++++++
>  5 files changed, 108 insertions(+)
>  create mode 100644 drivers/net/ethernet/intel/ice/ice_sf_eth.h
>
> diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.c b/drivers/net/ethernet/intel/ice/devlink/devlink.c
> index 3fb3a7e828a4..c1fe3726f6c0 100644
> --- a/drivers/net/ethernet/intel/ice/devlink/devlink.c
> +++ b/drivers/net/ethernet/intel/ice/devlink/devlink.c
> @@ -10,6 +10,7 @@
>  #include "ice_eswitch.h"
>  #include "ice_fw_update.h"
>  #include "ice_dcb_lib.h"
> +#include "ice_sf_eth.h"
>
>  /* context for devlink info version reporting */
>  struct ice_info_ctx {
> @@ -1282,6 +1283,8 @@ static const struct devlink_ops ice_devlink_ops = {
>         .port_new = ice_devlink_port_new,
>  };
>
> +static const struct devlink_ops ice_sf_devlink_ops;
> +
>  static int
>  ice_devlink_enable_roce_get(struct devlink *devlink, u32 id,
>                             struct devlink_param_gset_ctx *ctx)
> @@ -1419,6 +1422,7 @@ static void ice_devlink_free(void *devlink_ptr)
>   * Allocate a devlink instance for this device and return the private area as
>   * the PF structure. The devlink memory is kept track of through devres by
>   * adding an action to remove it when unwinding.
> + *
>   */
>  struct ice_pf *ice_allocate_pf(struct device *dev)
>  {
> @@ -1435,6 +1439,35 @@ struct ice_pf *ice_allocate_pf(struct device *dev)
>         return devlink_priv(devlink);
>  }
>
> +/**
> + * ice_allocate_sf - Allocate devlink and return SF structure pointer
> + * @dev: the device to allocate for
> + * @pf: pointer to the PF structure
> + *
> + * Allocate a devlink instance for SF.
> + *
> + * Return: void pointer to allocated memory
> + */
> +struct ice_sf_priv *ice_allocate_sf(struct device *dev, struct ice_pf *pf)
> +{
> +       struct devlink *devlink;
> +       int err;
> +
> +       devlink = devlink_alloc_ns(&ice_sf_devlink_ops,
> +                                  sizeof(struct ice_sf_priv),
> +                                  devlink_net(priv_to_devlink(pf)), dev);
> +       if (!devlink)
> +               return NULL;
> +
> +       err = devl_nested_devlink_set(priv_to_devlink(pf), devlink);
> +       if (err) {
> +               devlink_free(devlink);
> +               return ERR_PTR(err);
> +       }
> +
> +       return devlink_priv(devlink);
> +}
> +
>  /**
>   * ice_devlink_register - Register devlink interface for this PF
>   * @pf: the PF to register the devlink for.
> diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.h b/drivers/net/ethernet/intel/ice/devlink/devlink.h
> index d291c0e2e17b..1af3b0763fbb 100644
> --- a/drivers/net/ethernet/intel/ice/devlink/devlink.h
> +++ b/drivers/net/ethernet/intel/ice/devlink/devlink.h
> @@ -5,6 +5,7 @@
>  #define _ICE_DEVLINK_H_
>
>  struct ice_pf *ice_allocate_pf(struct device *dev);
> +struct ice_sf_priv *ice_allocate_sf(struct device *dev, struct ice_pf *pf);
>
>  void ice_devlink_register(struct ice_pf *pf);
>  void ice_devlink_unregister(struct ice_pf *pf);
> diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink_port.c b/drivers/net/ethernet/intel/ice/devlink/devlink_port.c
> index 812b306e9048..1355ea042f1d 100644
> --- a/drivers/net/ethernet/intel/ice/devlink/devlink_port.c
> +++ b/drivers/net/ethernet/intel/ice/devlink/devlink_port.c
> @@ -432,6 +432,56 @@ void ice_devlink_destroy_vf_port(struct ice_vf *vf)
>         devl_port_unregister(&vf->devlink_port);
>  }
>
> +/**
> + * ice_devlink_create_sf_dev_port - Register virtual port for a subfunction
> + * @sf_dev: the subfunction device to create a devlink port for
> + *
> + * Register virtual flavour devlink port for the subfunction auxiliary device
> + * created after activating a dynamically added devlink port.
> + *
> + * Return: zero on success or an error code on failure.
> + */
> +int ice_devlink_create_sf_dev_port(struct ice_sf_dev *sf_dev)
> +{
> +       struct devlink_port_attrs attrs = {};
> +       struct devlink_port *devlink_port;
> +       struct ice_dynamic_port *dyn_port;
[Kalesh] Try to maintain RCT order for variable declaration.
> +       struct devlink *devlink;
> +       struct ice_vsi *vsi;
> +       struct device *dev;
> +       struct ice_pf *pf;
> +       int err;
> +
> +       dyn_port = sf_dev->dyn_port;
> +       vsi = dyn_port->vsi;
> +       pf = dyn_port->pf;
> +       dev = ice_pf_to_dev(pf);
> +
> +       devlink_port = &sf_dev->priv->devlink_port;
> +
> +       attrs.flavour = DEVLINK_PORT_FLAVOUR_VIRTUAL;
> +
> +       devlink_port_attrs_set(devlink_port, &attrs);
> +       devlink = priv_to_devlink(sf_dev->priv);
> +
> +       err = devl_port_register(devlink, devlink_port, vsi->idx);
> +       if (err)
> +               dev_err(dev, "Failed to create virtual devlink port for auxiliary subfunction device");
> +
> +       return err;
> +}
> +
> +/**
> + * ice_devlink_destroy_sf_dev_port - Destroy virtual port for a subfunction
> + * @sf_dev: the subfunction device to create a devlink port for
> + *
> + * Unregisters the virtual port associated with this subfunction.
> + */
> +void ice_devlink_destroy_sf_dev_port(struct ice_sf_dev *sf_dev)
> +{
> +       devl_port_unregister(&sf_dev->priv->devlink_port);
> +}
> +
>  /**
>   * ice_dealloc_dynamic_port - Deallocate and remove a dynamic port
>   * @dyn_port: dynamic port instance to deallocate
> diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink_port.h b/drivers/net/ethernet/intel/ice/devlink/devlink_port.h
> index f20d7cc522a6..e4acd855d9f9 100644
> --- a/drivers/net/ethernet/intel/ice/devlink/devlink_port.h
> +++ b/drivers/net/ethernet/intel/ice/devlink/devlink_port.h
> @@ -5,6 +5,7 @@
>  #define _DEVLINK_PORT_H_
>
>  #include "../ice.h"
> +#include "../ice_sf_eth.h"
>
>  /**
>   * struct ice_dynamic_port - Track dynamically added devlink port instance
> @@ -33,6 +34,8 @@ int ice_devlink_create_vf_port(struct ice_vf *vf);
>  void ice_devlink_destroy_vf_port(struct ice_vf *vf);
>  int ice_devlink_create_sf_port(struct ice_dynamic_port *dyn_port);
>  void ice_devlink_destroy_sf_port(struct ice_dynamic_port *dyn_port);
> +int ice_devlink_create_sf_dev_port(struct ice_sf_dev *sf_dev);
> +void ice_devlink_destroy_sf_dev_port(struct ice_sf_dev *sf_dev);
>
>  #define ice_devlink_port_to_dyn(p) \
>         container_of(port, struct ice_dynamic_port, devlink_port)
> diff --git a/drivers/net/ethernet/intel/ice/ice_sf_eth.h b/drivers/net/ethernet/intel/ice/ice_sf_eth.h
> new file mode 100644
> index 000000000000..a08f8b2bceef
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/ice/ice_sf_eth.h
> @@ -0,0 +1,21 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (c) 2024, Intel Corporation. */
> +
> +#ifndef _ICE_SF_ETH_H_
> +#define _ICE_SF_ETH_H_
> +
> +#include <linux/auxiliary_bus.h>
> +#include "ice.h"
> +
> +struct ice_sf_dev {
> +       struct auxiliary_device adev;
> +       struct ice_dynamic_port *dyn_port;
> +       struct ice_sf_priv *priv;
> +};
> +
> +struct ice_sf_priv {
> +       struct ice_sf_dev *dev;
> +       struct devlink_port devlink_port;
> +};
> +
> +#endif /* _ICE_SF_ETH_H_ */
> --
> 2.42.0
>
>
Michal Swiatkowski May 13, 2024, 10:22 a.m. UTC | #2
On Mon, May 13, 2024 at 02:55:48PM +0530, Kalesh Anakkur Purayil wrote:
> On Mon, May 13, 2024 at 2:03 PM Michal Swiatkowski
> <michal.swiatkowski@linux.intel.com> wrote:
> >
> > From: Piotr Raczynski <piotr.raczynski@intel.com>
> >
> > Make devlink allocation function generic to use it for PF and for SF.
> >
> > Add function for SF devlink port creation. It will be used in next
> > patch.
> >
> > Create header file for subfunction device. Define subfunction device
> > structure there as it is needed for devlink allocation and port
> > creation.
> >
> > Signed-off-by: Piotr Raczynski <piotr.raczynski@intel.com>
> > Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> > ---
> >  .../net/ethernet/intel/ice/devlink/devlink.c  | 33 ++++++++++++
> >  .../net/ethernet/intel/ice/devlink/devlink.h  |  1 +
> >  .../ethernet/intel/ice/devlink/devlink_port.c | 50 +++++++++++++++++++
> >  .../ethernet/intel/ice/devlink/devlink_port.h |  3 ++
> >  drivers/net/ethernet/intel/ice/ice_sf_eth.h   | 21 ++++++++
> >  5 files changed, 108 insertions(+)
> >  create mode 100644 drivers/net/ethernet/intel/ice/ice_sf_eth.h
> >
> > diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.c b/drivers/net/ethernet/intel/ice/devlink/devlink.c
> > index 3fb3a7e828a4..c1fe3726f6c0 100644
> > --- a/drivers/net/ethernet/intel/ice/devlink/devlink.c
> > +++ b/drivers/net/ethernet/intel/ice/devlink/devlink.c
> > @@ -10,6 +10,7 @@
> >  #include "ice_eswitch.h"
> >  #include "ice_fw_update.h"
> >  #include "ice_dcb_lib.h"
> > +#include "ice_sf_eth.h"
> >
> >  /* context for devlink info version reporting */
> >  struct ice_info_ctx {
> > @@ -1282,6 +1283,8 @@ static const struct devlink_ops ice_devlink_ops = {
> >         .port_new = ice_devlink_port_new,
> >  };
> >
> > +static const struct devlink_ops ice_sf_devlink_ops;
> > +
> >  static int
> >  ice_devlink_enable_roce_get(struct devlink *devlink, u32 id,
> >                             struct devlink_param_gset_ctx *ctx)
> > @@ -1419,6 +1422,7 @@ static void ice_devlink_free(void *devlink_ptr)
> >   * Allocate a devlink instance for this device and return the private area as
> >   * the PF structure. The devlink memory is kept track of through devres by
> >   * adding an action to remove it when unwinding.
> > + *
> >   */
> >  struct ice_pf *ice_allocate_pf(struct device *dev)
> >  {
> > @@ -1435,6 +1439,35 @@ struct ice_pf *ice_allocate_pf(struct device *dev)
> >         return devlink_priv(devlink);
> >  }
> >
> > +/**
> > + * ice_allocate_sf - Allocate devlink and return SF structure pointer
> > + * @dev: the device to allocate for
> > + * @pf: pointer to the PF structure
> > + *
> > + * Allocate a devlink instance for SF.
> > + *
> > + * Return: void pointer to allocated memory
> > + */
> > +struct ice_sf_priv *ice_allocate_sf(struct device *dev, struct ice_pf *pf)
> > +{
> > +       struct devlink *devlink;
> > +       int err;
> > +
> > +       devlink = devlink_alloc_ns(&ice_sf_devlink_ops,
> > +                                  sizeof(struct ice_sf_priv),
> > +                                  devlink_net(priv_to_devlink(pf)), dev);
> > +       if (!devlink)
> > +               return NULL;
> > +
> > +       err = devl_nested_devlink_set(priv_to_devlink(pf), devlink);
> > +       if (err) {
> > +               devlink_free(devlink);
> > +               return ERR_PTR(err);
> > +       }
> > +
> > +       return devlink_priv(devlink);
> > +}
> > +
> >  /**
> >   * ice_devlink_register - Register devlink interface for this PF
> >   * @pf: the PF to register the devlink for.
> > diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.h b/drivers/net/ethernet/intel/ice/devlink/devlink.h
> > index d291c0e2e17b..1af3b0763fbb 100644
> > --- a/drivers/net/ethernet/intel/ice/devlink/devlink.h
> > +++ b/drivers/net/ethernet/intel/ice/devlink/devlink.h
> > @@ -5,6 +5,7 @@
> >  #define _ICE_DEVLINK_H_
> >
> >  struct ice_pf *ice_allocate_pf(struct device *dev);
> > +struct ice_sf_priv *ice_allocate_sf(struct device *dev, struct ice_pf *pf);
> >
> >  void ice_devlink_register(struct ice_pf *pf);
> >  void ice_devlink_unregister(struct ice_pf *pf);
> > diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink_port.c b/drivers/net/ethernet/intel/ice/devlink/devlink_port.c
> > index 812b306e9048..1355ea042f1d 100644
> > --- a/drivers/net/ethernet/intel/ice/devlink/devlink_port.c
> > +++ b/drivers/net/ethernet/intel/ice/devlink/devlink_port.c
> > @@ -432,6 +432,56 @@ void ice_devlink_destroy_vf_port(struct ice_vf *vf)
> >         devl_port_unregister(&vf->devlink_port);
> >  }
> >
> > +/**
> > + * ice_devlink_create_sf_dev_port - Register virtual port for a subfunction
> > + * @sf_dev: the subfunction device to create a devlink port for
> > + *
> > + * Register virtual flavour devlink port for the subfunction auxiliary device
> > + * created after activating a dynamically added devlink port.
> > + *
> > + * Return: zero on success or an error code on failure.
> > + */
> > +int ice_devlink_create_sf_dev_port(struct ice_sf_dev *sf_dev)
> > +{
> > +       struct devlink_port_attrs attrs = {};
> > +       struct devlink_port *devlink_port;
> > +       struct ice_dynamic_port *dyn_port;
> [Kalesh] Try to maintain RCT order for variable declaration.

Maybe I don't understand RCT order correctly, but based on my
understanding it is fine. Which declaration here break RCT order?

Do you mean that ice_dynamic_port is longer than devlink_port and should
be moved up? Didn't know that RCT is also applied to inner part of
declaration. If there will be more comments I will move it in another
spin.

> > +       struct devlink *devlink;
> > +       struct ice_vsi *vsi;
> > +       struct device *dev;
> > +       struct ice_pf *pf;
> > +       int err;
> > +
> > +       dyn_port = sf_dev->dyn_port;
> > +       vsi = dyn_port->vsi;
> > +       pf = dyn_port->pf;
> > +       dev = ice_pf_to_dev(pf);
> > +
> > +       devlink_port = &sf_dev->priv->devlink_port;
> > +
> > +       attrs.flavour = DEVLINK_PORT_FLAVOUR_VIRTUAL;
> > +
> > +       devlink_port_attrs_set(devlink_port, &attrs);
> > +       devlink = priv_to_devlink(sf_dev->priv);
> > +
> > +       err = devl_port_register(devlink, devlink_port, vsi->idx);
> > +       if (err)
> > +               dev_err(dev, "Failed to create virtual devlink port for auxiliary subfunction device");
> > +
> > +       return err;
> > +}
> > +
> > +/**
> > + * ice_devlink_destroy_sf_dev_port - Destroy virtual port for a subfunction
> > + * @sf_dev: the subfunction device to create a devlink port for
> > + *
> > + * Unregisters the virtual port associated with this subfunction.
> > + */
> > +void ice_devlink_destroy_sf_dev_port(struct ice_sf_dev *sf_dev)
> > +{
> > +       devl_port_unregister(&sf_dev->priv->devlink_port);
> > +}
> > +
> >  /**
> >   * ice_dealloc_dynamic_port - Deallocate and remove a dynamic port
> >   * @dyn_port: dynamic port instance to deallocate
> > diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink_port.h b/drivers/net/ethernet/intel/ice/devlink/devlink_port.h
> > index f20d7cc522a6..e4acd855d9f9 100644
> > --- a/drivers/net/ethernet/intel/ice/devlink/devlink_port.h
> > +++ b/drivers/net/ethernet/intel/ice/devlink/devlink_port.h
> > @@ -5,6 +5,7 @@
> >  #define _DEVLINK_PORT_H_
> >
> >  #include "../ice.h"
> > +#include "../ice_sf_eth.h"
> >
> >  /**
> >   * struct ice_dynamic_port - Track dynamically added devlink port instance
> > @@ -33,6 +34,8 @@ int ice_devlink_create_vf_port(struct ice_vf *vf);
> >  void ice_devlink_destroy_vf_port(struct ice_vf *vf);
> >  int ice_devlink_create_sf_port(struct ice_dynamic_port *dyn_port);
> >  void ice_devlink_destroy_sf_port(struct ice_dynamic_port *dyn_port);
> > +int ice_devlink_create_sf_dev_port(struct ice_sf_dev *sf_dev);
> > +void ice_devlink_destroy_sf_dev_port(struct ice_sf_dev *sf_dev);
> >
> >  #define ice_devlink_port_to_dyn(p) \
> >         container_of(port, struct ice_dynamic_port, devlink_port)
> > diff --git a/drivers/net/ethernet/intel/ice/ice_sf_eth.h b/drivers/net/ethernet/intel/ice/ice_sf_eth.h
> > new file mode 100644
> > index 000000000000..a08f8b2bceef
> > --- /dev/null
> > +++ b/drivers/net/ethernet/intel/ice/ice_sf_eth.h
> > @@ -0,0 +1,21 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/* Copyright (c) 2024, Intel Corporation. */
> > +
> > +#ifndef _ICE_SF_ETH_H_
> > +#define _ICE_SF_ETH_H_
> > +
> > +#include <linux/auxiliary_bus.h>
> > +#include "ice.h"
> > +
> > +struct ice_sf_dev {
> > +       struct auxiliary_device adev;
> > +       struct ice_dynamic_port *dyn_port;
> > +       struct ice_sf_priv *priv;
> > +};
> > +
> > +struct ice_sf_priv {
> > +       struct ice_sf_dev *dev;
> > +       struct devlink_port devlink_port;
> > +};
> > +
> > +#endif /* _ICE_SF_ETH_H_ */
> > --
> > 2.42.0
> >
> >
> 
> 
> -- 
> Regards,
> Kalesh A P
Kalesh Anakkur Purayil May 13, 2024, 12:01 p.m. UTC | #3
On Mon, May 13, 2024 at 3:54 PM Michal Swiatkowski
<michal.swiatkowski@linux.intel.com> wrote:
>
> On Mon, May 13, 2024 at 02:55:48PM +0530, Kalesh Anakkur Purayil wrote:
> > On Mon, May 13, 2024 at 2:03 PM Michal Swiatkowski
> > <michal.swiatkowski@linux.intel.com> wrote:
> > >
> > > From: Piotr Raczynski <piotr.raczynski@intel.com>
> > >
> > > Make devlink allocation function generic to use it for PF and for SF.
> > >
> > > Add function for SF devlink port creation. It will be used in next
> > > patch.
> > >
> > > Create header file for subfunction device. Define subfunction device
> > > structure there as it is needed for devlink allocation and port
> > > creation.
> > >
> > > Signed-off-by: Piotr Raczynski <piotr.raczynski@intel.com>
> > > Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> > > ---
> > >  .../net/ethernet/intel/ice/devlink/devlink.c  | 33 ++++++++++++
> > >  .../net/ethernet/intel/ice/devlink/devlink.h  |  1 +
> > >  .../ethernet/intel/ice/devlink/devlink_port.c | 50 +++++++++++++++++++
> > >  .../ethernet/intel/ice/devlink/devlink_port.h |  3 ++
> > >  drivers/net/ethernet/intel/ice/ice_sf_eth.h   | 21 ++++++++
> > >  5 files changed, 108 insertions(+)
> > >  create mode 100644 drivers/net/ethernet/intel/ice/ice_sf_eth.h
> > >
> > > diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.c b/drivers/net/ethernet/intel/ice/devlink/devlink.c
> > > index 3fb3a7e828a4..c1fe3726f6c0 100644
> > > --- a/drivers/net/ethernet/intel/ice/devlink/devlink.c
> > > +++ b/drivers/net/ethernet/intel/ice/devlink/devlink.c
> > > @@ -10,6 +10,7 @@
> > >  #include "ice_eswitch.h"
> > >  #include "ice_fw_update.h"
> > >  #include "ice_dcb_lib.h"
> > > +#include "ice_sf_eth.h"
> > >
> > >  /* context for devlink info version reporting */
> > >  struct ice_info_ctx {
> > > @@ -1282,6 +1283,8 @@ static const struct devlink_ops ice_devlink_ops = {
> > >         .port_new = ice_devlink_port_new,
> > >  };
> > >
> > > +static const struct devlink_ops ice_sf_devlink_ops;
> > > +
> > >  static int
> > >  ice_devlink_enable_roce_get(struct devlink *devlink, u32 id,
> > >                             struct devlink_param_gset_ctx *ctx)
> > > @@ -1419,6 +1422,7 @@ static void ice_devlink_free(void *devlink_ptr)
> > >   * Allocate a devlink instance for this device and return the private area as
> > >   * the PF structure. The devlink memory is kept track of through devres by
> > >   * adding an action to remove it when unwinding.
> > > + *
> > >   */
> > >  struct ice_pf *ice_allocate_pf(struct device *dev)
> > >  {
> > > @@ -1435,6 +1439,35 @@ struct ice_pf *ice_allocate_pf(struct device *dev)
> > >         return devlink_priv(devlink);
> > >  }
> > >
> > > +/**
> > > + * ice_allocate_sf - Allocate devlink and return SF structure pointer
> > > + * @dev: the device to allocate for
> > > + * @pf: pointer to the PF structure
> > > + *
> > > + * Allocate a devlink instance for SF.
> > > + *
> > > + * Return: void pointer to allocated memory
> > > + */
> > > +struct ice_sf_priv *ice_allocate_sf(struct device *dev, struct ice_pf *pf)
> > > +{
> > > +       struct devlink *devlink;
> > > +       int err;
> > > +
> > > +       devlink = devlink_alloc_ns(&ice_sf_devlink_ops,
> > > +                                  sizeof(struct ice_sf_priv),
> > > +                                  devlink_net(priv_to_devlink(pf)), dev);
> > > +       if (!devlink)
> > > +               return NULL;
> > > +
> > > +       err = devl_nested_devlink_set(priv_to_devlink(pf), devlink);
> > > +       if (err) {
> > > +               devlink_free(devlink);
> > > +               return ERR_PTR(err);
> > > +       }
> > > +
> > > +       return devlink_priv(devlink);
> > > +}
> > > +
> > >  /**
> > >   * ice_devlink_register - Register devlink interface for this PF
> > >   * @pf: the PF to register the devlink for.
> > > diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.h b/drivers/net/ethernet/intel/ice/devlink/devlink.h
> > > index d291c0e2e17b..1af3b0763fbb 100644
> > > --- a/drivers/net/ethernet/intel/ice/devlink/devlink.h
> > > +++ b/drivers/net/ethernet/intel/ice/devlink/devlink.h
> > > @@ -5,6 +5,7 @@
> > >  #define _ICE_DEVLINK_H_
> > >
> > >  struct ice_pf *ice_allocate_pf(struct device *dev);
> > > +struct ice_sf_priv *ice_allocate_sf(struct device *dev, struct ice_pf *pf);
> > >
> > >  void ice_devlink_register(struct ice_pf *pf);
> > >  void ice_devlink_unregister(struct ice_pf *pf);
> > > diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink_port.c b/drivers/net/ethernet/intel/ice/devlink/devlink_port.c
> > > index 812b306e9048..1355ea042f1d 100644
> > > --- a/drivers/net/ethernet/intel/ice/devlink/devlink_port.c
> > > +++ b/drivers/net/ethernet/intel/ice/devlink/devlink_port.c
> > > @@ -432,6 +432,56 @@ void ice_devlink_destroy_vf_port(struct ice_vf *vf)
> > >         devl_port_unregister(&vf->devlink_port);
> > >  }
> > >
> > > +/**
> > > + * ice_devlink_create_sf_dev_port - Register virtual port for a subfunction
> > > + * @sf_dev: the subfunction device to create a devlink port for
> > > + *
> > > + * Register virtual flavour devlink port for the subfunction auxiliary device
> > > + * created after activating a dynamically added devlink port.
> > > + *
> > > + * Return: zero on success or an error code on failure.
> > > + */
> > > +int ice_devlink_create_sf_dev_port(struct ice_sf_dev *sf_dev)
> > > +{
> > > +       struct devlink_port_attrs attrs = {};
> > > +       struct devlink_port *devlink_port;
> > > +       struct ice_dynamic_port *dyn_port;
> > [Kalesh] Try to maintain RCT order for variable declaration.
>
> Maybe I don't understand RCT order correctly, but based on my
> understanding it is fine. Which declaration here break RCT order?
>
> Do you mean that ice_dynamic_port is longer than devlink_port and should
> be moved up?
[Kalesh] Yes, longest line to shortest for local variable declarations
in new Networking code.
Didn't know that RCT is also applied to inner part of
> declaration. If there will be more comments I will move it in another
> spin.
[Kalesh] Thanks
>
> > > +       struct devlink *devlink;
> > > +       struct ice_vsi *vsi;
> > > +       struct device *dev;
> > > +       struct ice_pf *pf;
> > > +       int err;
> > > +
> > > +       dyn_port = sf_dev->dyn_port;
> > > +       vsi = dyn_port->vsi;
> > > +       pf = dyn_port->pf;
> > > +       dev = ice_pf_to_dev(pf);
> > > +
> > > +       devlink_port = &sf_dev->priv->devlink_port;
> > > +
> > > +       attrs.flavour = DEVLINK_PORT_FLAVOUR_VIRTUAL;
> > > +
> > > +       devlink_port_attrs_set(devlink_port, &attrs);
> > > +       devlink = priv_to_devlink(sf_dev->priv);
> > > +
> > > +       err = devl_port_register(devlink, devlink_port, vsi->idx);
> > > +       if (err)
> > > +               dev_err(dev, "Failed to create virtual devlink port for auxiliary subfunction device");
> > > +
> > > +       return err;
> > > +}
> > > +
> > > +/**
> > > + * ice_devlink_destroy_sf_dev_port - Destroy virtual port for a subfunction
> > > + * @sf_dev: the subfunction device to create a devlink port for
> > > + *
> > > + * Unregisters the virtual port associated with this subfunction.
> > > + */
> > > +void ice_devlink_destroy_sf_dev_port(struct ice_sf_dev *sf_dev)
> > > +{
> > > +       devl_port_unregister(&sf_dev->priv->devlink_port);
> > > +}
> > > +
> > >  /**
> > >   * ice_dealloc_dynamic_port - Deallocate and remove a dynamic port
> > >   * @dyn_port: dynamic port instance to deallocate
> > > diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink_port.h b/drivers/net/ethernet/intel/ice/devlink/devlink_port.h
> > > index f20d7cc522a6..e4acd855d9f9 100644
> > > --- a/drivers/net/ethernet/intel/ice/devlink/devlink_port.h
> > > +++ b/drivers/net/ethernet/intel/ice/devlink/devlink_port.h
> > > @@ -5,6 +5,7 @@
> > >  #define _DEVLINK_PORT_H_
> > >
> > >  #include "../ice.h"
> > > +#include "../ice_sf_eth.h"
> > >
> > >  /**
> > >   * struct ice_dynamic_port - Track dynamically added devlink port instance
> > > @@ -33,6 +34,8 @@ int ice_devlink_create_vf_port(struct ice_vf *vf);
> > >  void ice_devlink_destroy_vf_port(struct ice_vf *vf);
> > >  int ice_devlink_create_sf_port(struct ice_dynamic_port *dyn_port);
> > >  void ice_devlink_destroy_sf_port(struct ice_dynamic_port *dyn_port);
> > > +int ice_devlink_create_sf_dev_port(struct ice_sf_dev *sf_dev);
> > > +void ice_devlink_destroy_sf_dev_port(struct ice_sf_dev *sf_dev);
> > >
> > >  #define ice_devlink_port_to_dyn(p) \
> > >         container_of(port, struct ice_dynamic_port, devlink_port)
> > > diff --git a/drivers/net/ethernet/intel/ice/ice_sf_eth.h b/drivers/net/ethernet/intel/ice/ice_sf_eth.h
> > > new file mode 100644
> > > index 000000000000..a08f8b2bceef
> > > --- /dev/null
> > > +++ b/drivers/net/ethernet/intel/ice/ice_sf_eth.h
> > > @@ -0,0 +1,21 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +/* Copyright (c) 2024, Intel Corporation. */
> > > +
> > > +#ifndef _ICE_SF_ETH_H_
> > > +#define _ICE_SF_ETH_H_
> > > +
> > > +#include <linux/auxiliary_bus.h>
> > > +#include "ice.h"
> > > +
> > > +struct ice_sf_dev {
> > > +       struct auxiliary_device adev;
> > > +       struct ice_dynamic_port *dyn_port;
> > > +       struct ice_sf_priv *priv;
> > > +};
> > > +
> > > +struct ice_sf_priv {
> > > +       struct ice_sf_dev *dev;
> > > +       struct devlink_port devlink_port;
> > > +};
> > > +
> > > +#endif /* _ICE_SF_ETH_H_ */
> > > --
> > > 2.42.0
> > >
> > >
> >
> >
> > --
> > Regards,
> > Kalesh A P
>
>
Jacob Keller May 13, 2024, 9:37 p.m. UTC | #4
> -----Original Message-----
> From: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> Sent: Monday, May 13, 2024 3:23 AM
> To: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@broadcom.com>
> Cc: shayd@nvidia.com; Fijalkowski, Maciej <maciej.fijalkowski@intel.com>;
> Polchlopek, Mateusz <mateusz.polchlopek@intel.com>; netdev@vger.kernel.org;
> jiri@nvidia.com; Kubiak, Michal <michal.kubiak@intel.com>; intel-wired-
> lan@lists.osuosl.org; pio.raczynski@gmail.com; Samudrala, Sridhar
> <sridhar.samudrala@intel.com>; Keller, Jacob E <jacob.e.keller@intel.com>;
> Drewek, Wojciech <wojciech.drewek@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>
> Subject: Re: [Intel-wired-lan] [iwl-next v2 05/15] ice: allocate devlink for
> subfunction
> 
> On Mon, May 13, 2024 at 02:55:48PM +0530, Kalesh Anakkur Purayil wrote:
> > On Mon, May 13, 2024 at 2:03 PM Michal Swiatkowski
> > > +       struct devlink_port_attrs attrs = {};
> > > +       struct devlink_port *devlink_port;
> > > +       struct ice_dynamic_port *dyn_port;
> > [Kalesh] Try to maintain RCT order for variable declaration.
> 
> Maybe I don't understand RCT order correctly, but based on my
> understanding it is fine. Which declaration here break RCT order?
> 
> Do you mean that ice_dynamic_port is longer than devlink_port and should
> be moved up? Didn't know that RCT is also applied to inner part of
> declaration. If there will be more comments I will move it in another
> spin.
> 

RCT (Reverse Christmas Tree) order would be to put the longest declaration line first then the rest in order down to shortest. RCT is preferred over using initializers in the case where initializers would add a dependency that forces a non-RCT ordering. In that case, you would delay initialization to a block after the declarations.

So here, you would put dyn_port first, then attrs, then devlink_port.

Thanks,
Jake
Przemek Kitszel May 14, 2024, 6:36 a.m. UTC | #5
On 5/13/24 23:37, Keller, Jacob E wrote:
> 
> 
>> -----Original Message-----
>> From: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
>> Sent: Monday, May 13, 2024 3:23 AM
>> To: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@broadcom.com>
>> Cc: shayd@nvidia.com; Fijalkowski, Maciej <maciej.fijalkowski@intel.com>;
>> Polchlopek, Mateusz <mateusz.polchlopek@intel.com>; netdev@vger.kernel.org;
>> jiri@nvidia.com; Kubiak, Michal <michal.kubiak@intel.com>; intel-wired-
>> lan@lists.osuosl.org; pio.raczynski@gmail.com; Samudrala, Sridhar
>> <sridhar.samudrala@intel.com>; Keller, Jacob E <jacob.e.keller@intel.com>;
>> Drewek, Wojciech <wojciech.drewek@intel.com>; Kitszel, Przemyslaw
>> <przemyslaw.kitszel@intel.com>
>> Subject: Re: [Intel-wired-lan] [iwl-next v2 05/15] ice: allocate devlink for
>> subfunction
>>
>> On Mon, May 13, 2024 at 02:55:48PM +0530, Kalesh Anakkur Purayil wrote:
>>> On Mon, May 13, 2024 at 2:03 PM Michal Swiatkowski
>>>> +       struct devlink_port_attrs attrs = {};
>>>> +       struct devlink_port *devlink_port;
>>>> +       struct ice_dynamic_port *dyn_port;
>>> [Kalesh] Try to maintain RCT order for variable declaration.
>>
>> Maybe I don't understand RCT order correctly, but based on my
>> understanding it is fine. Which declaration here break RCT order?
>>
>> Do you mean that ice_dynamic_port is longer than devlink_port and should
>> be moved up? Didn't know that RCT is also applied to inner part of
>> declaration. If there will be more comments I will move it in another
>> spin.
>>
> 
> RCT (Reverse Christmas Tree) order would be to put the longest declaration line first then the rest in order down to shortest. RCT is preferred over using initializers in the case where initializers would add a dependency that forces a non-RCT ordering. In that case, you would delay initialization to a block after the declarations.
> 
> So here, you would put dyn_port first, then attrs, then devlink_port.
> 
> Thanks,
> Jake
> 

Documentation/process/maintainer-netdev.rst says nothing about how to
resolve ties, and we sort by byte-count of given line. For me it means
that author is free to decide.
(Thank ASCII gods that we don't accept full width CJK chars;)

Here order is correct, swapping devlink_port and dyn_port lines will
be equally correct.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.c b/drivers/net/ethernet/intel/ice/devlink/devlink.c
index 3fb3a7e828a4..c1fe3726f6c0 100644
--- a/drivers/net/ethernet/intel/ice/devlink/devlink.c
+++ b/drivers/net/ethernet/intel/ice/devlink/devlink.c
@@ -10,6 +10,7 @@ 
 #include "ice_eswitch.h"
 #include "ice_fw_update.h"
 #include "ice_dcb_lib.h"
+#include "ice_sf_eth.h"
 
 /* context for devlink info version reporting */
 struct ice_info_ctx {
@@ -1282,6 +1283,8 @@  static const struct devlink_ops ice_devlink_ops = {
 	.port_new = ice_devlink_port_new,
 };
 
+static const struct devlink_ops ice_sf_devlink_ops;
+
 static int
 ice_devlink_enable_roce_get(struct devlink *devlink, u32 id,
 			    struct devlink_param_gset_ctx *ctx)
@@ -1419,6 +1422,7 @@  static void ice_devlink_free(void *devlink_ptr)
  * Allocate a devlink instance for this device and return the private area as
  * the PF structure. The devlink memory is kept track of through devres by
  * adding an action to remove it when unwinding.
+ *
  */
 struct ice_pf *ice_allocate_pf(struct device *dev)
 {
@@ -1435,6 +1439,35 @@  struct ice_pf *ice_allocate_pf(struct device *dev)
 	return devlink_priv(devlink);
 }
 
+/**
+ * ice_allocate_sf - Allocate devlink and return SF structure pointer
+ * @dev: the device to allocate for
+ * @pf: pointer to the PF structure
+ *
+ * Allocate a devlink instance for SF.
+ *
+ * Return: void pointer to allocated memory
+ */
+struct ice_sf_priv *ice_allocate_sf(struct device *dev, struct ice_pf *pf)
+{
+	struct devlink *devlink;
+	int err;
+
+	devlink = devlink_alloc_ns(&ice_sf_devlink_ops,
+				   sizeof(struct ice_sf_priv),
+				   devlink_net(priv_to_devlink(pf)), dev);
+	if (!devlink)
+		return NULL;
+
+	err = devl_nested_devlink_set(priv_to_devlink(pf), devlink);
+	if (err) {
+		devlink_free(devlink);
+		return ERR_PTR(err);
+	}
+
+	return devlink_priv(devlink);
+}
+
 /**
  * ice_devlink_register - Register devlink interface for this PF
  * @pf: the PF to register the devlink for.
diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.h b/drivers/net/ethernet/intel/ice/devlink/devlink.h
index d291c0e2e17b..1af3b0763fbb 100644
--- a/drivers/net/ethernet/intel/ice/devlink/devlink.h
+++ b/drivers/net/ethernet/intel/ice/devlink/devlink.h
@@ -5,6 +5,7 @@ 
 #define _ICE_DEVLINK_H_
 
 struct ice_pf *ice_allocate_pf(struct device *dev);
+struct ice_sf_priv *ice_allocate_sf(struct device *dev, struct ice_pf *pf);
 
 void ice_devlink_register(struct ice_pf *pf);
 void ice_devlink_unregister(struct ice_pf *pf);
diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink_port.c b/drivers/net/ethernet/intel/ice/devlink/devlink_port.c
index 812b306e9048..1355ea042f1d 100644
--- a/drivers/net/ethernet/intel/ice/devlink/devlink_port.c
+++ b/drivers/net/ethernet/intel/ice/devlink/devlink_port.c
@@ -432,6 +432,56 @@  void ice_devlink_destroy_vf_port(struct ice_vf *vf)
 	devl_port_unregister(&vf->devlink_port);
 }
 
+/**
+ * ice_devlink_create_sf_dev_port - Register virtual port for a subfunction
+ * @sf_dev: the subfunction device to create a devlink port for
+ *
+ * Register virtual flavour devlink port for the subfunction auxiliary device
+ * created after activating a dynamically added devlink port.
+ *
+ * Return: zero on success or an error code on failure.
+ */
+int ice_devlink_create_sf_dev_port(struct ice_sf_dev *sf_dev)
+{
+	struct devlink_port_attrs attrs = {};
+	struct devlink_port *devlink_port;
+	struct ice_dynamic_port *dyn_port;
+	struct devlink *devlink;
+	struct ice_vsi *vsi;
+	struct device *dev;
+	struct ice_pf *pf;
+	int err;
+
+	dyn_port = sf_dev->dyn_port;
+	vsi = dyn_port->vsi;
+	pf = dyn_port->pf;
+	dev = ice_pf_to_dev(pf);
+
+	devlink_port = &sf_dev->priv->devlink_port;
+
+	attrs.flavour = DEVLINK_PORT_FLAVOUR_VIRTUAL;
+
+	devlink_port_attrs_set(devlink_port, &attrs);
+	devlink = priv_to_devlink(sf_dev->priv);
+
+	err = devl_port_register(devlink, devlink_port, vsi->idx);
+	if (err)
+		dev_err(dev, "Failed to create virtual devlink port for auxiliary subfunction device");
+
+	return err;
+}
+
+/**
+ * ice_devlink_destroy_sf_dev_port - Destroy virtual port for a subfunction
+ * @sf_dev: the subfunction device to create a devlink port for
+ *
+ * Unregisters the virtual port associated with this subfunction.
+ */
+void ice_devlink_destroy_sf_dev_port(struct ice_sf_dev *sf_dev)
+{
+	devl_port_unregister(&sf_dev->priv->devlink_port);
+}
+
 /**
  * ice_dealloc_dynamic_port - Deallocate and remove a dynamic port
  * @dyn_port: dynamic port instance to deallocate
diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink_port.h b/drivers/net/ethernet/intel/ice/devlink/devlink_port.h
index f20d7cc522a6..e4acd855d9f9 100644
--- a/drivers/net/ethernet/intel/ice/devlink/devlink_port.h
+++ b/drivers/net/ethernet/intel/ice/devlink/devlink_port.h
@@ -5,6 +5,7 @@ 
 #define _DEVLINK_PORT_H_
 
 #include "../ice.h"
+#include "../ice_sf_eth.h"
 
 /**
  * struct ice_dynamic_port - Track dynamically added devlink port instance
@@ -33,6 +34,8 @@  int ice_devlink_create_vf_port(struct ice_vf *vf);
 void ice_devlink_destroy_vf_port(struct ice_vf *vf);
 int ice_devlink_create_sf_port(struct ice_dynamic_port *dyn_port);
 void ice_devlink_destroy_sf_port(struct ice_dynamic_port *dyn_port);
+int ice_devlink_create_sf_dev_port(struct ice_sf_dev *sf_dev);
+void ice_devlink_destroy_sf_dev_port(struct ice_sf_dev *sf_dev);
 
 #define ice_devlink_port_to_dyn(p) \
 	container_of(port, struct ice_dynamic_port, devlink_port)
diff --git a/drivers/net/ethernet/intel/ice/ice_sf_eth.h b/drivers/net/ethernet/intel/ice/ice_sf_eth.h
new file mode 100644
index 000000000000..a08f8b2bceef
--- /dev/null
+++ b/drivers/net/ethernet/intel/ice/ice_sf_eth.h
@@ -0,0 +1,21 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2024, Intel Corporation. */
+
+#ifndef _ICE_SF_ETH_H_
+#define _ICE_SF_ETH_H_
+
+#include <linux/auxiliary_bus.h>
+#include "ice.h"
+
+struct ice_sf_dev {
+	struct auxiliary_device adev;
+	struct ice_dynamic_port *dyn_port;
+	struct ice_sf_priv *priv;
+};
+
+struct ice_sf_priv {
+	struct ice_sf_dev *dev;
+	struct devlink_port devlink_port;
+};
+
+#endif /* _ICE_SF_ETH_H_ */