| Message ID | 20240513083735.54791-6-michal.swiatkowski@linux.intel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | ice: support devlink subfunction | expand |
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 > >
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
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 > >
> -----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
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 --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_ */