On Fri, Apr 18, 2025 at 05:14:24PM +0000, Ertman, David M wrote: > > -----Original Message----- > > From: Intel-wired-lan <[email protected]> On Behalf Of > > Simon Horman > > Sent: Thursday, April 17, 2025 4:22 AM > > To: Nikolova, Tatyana E <[email protected]> > > Cc: [email protected]; [email protected]; [email protected]; > > linux- > > [email protected]; [email protected] > > Subject: Re: [Intel-wired-lan] [iwl-next v5 5/5] iidc/ice/irdma: Update IDC > > to > > support multiple consumers > > > > On Tue, Apr 15, 2025 at 09:15:49PM -0500, Tatyana Nikolova wrote: > > > From: Dave Ertman <[email protected]> > > > > > > In preparation of supporting more than a single core PCI driver > > > for RDMA, move ice specific structs like qset_params, qos_info > > > and qos_params from iidc_rdma.h to iidc_rdma_ice.h. > > > > > > Previously, the ice driver was just exporting its entire PF struct > > > to the auxiliary driver, but since each core driver will have its own > > > different PF struct, implement a universal struct that all core drivers > > > can provide to the auxiliary driver through the probe call. > > > > > > Reviewed-by: Przemek Kitszel <[email protected]> > > > Signed-off-by: Dave Ertman <[email protected]> > > > Co-developed-by: Mustafa Ismail <[email protected]> > > > Signed-off-by: Mustafa Ismail <[email protected]> > > > Co-developed-by: Shiraz Saleem <[email protected]> > > > Signed-off-by: Shiraz Saleem <[email protected]> > > > Co-developed-by: Tatyana Nikolova <[email protected]> > > > Signed-off-by: Tatyana Nikolova <[email protected]> > > > > ... > > > > > diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.c > > b/drivers/net/ethernet/intel/ice/devlink/devlink.c > > > index fcb199efbea5..4af60e2f37df 100644 > > > --- a/drivers/net/ethernet/intel/ice/devlink/devlink.c > > > +++ b/drivers/net/ethernet/intel/ice/devlink/devlink.c > > > @@ -1339,8 +1339,13 @@ ice_devlink_enable_roce_get(struct devlink > > *devlink, u32 id, > > > struct devlink_param_gset_ctx *ctx) > > > { > > > struct ice_pf *pf = devlink_priv(devlink); > > > + struct iidc_rdma_core_dev_info *cdev; > > > > > > - ctx->val.vbool = pf->rdma_mode & IIDC_RDMA_PROTOCOL_ROCEV2 > > ? true : false; > > > + cdev = pf->cdev_info; > > > + if (!cdev) > > > + return -ENODEV; > > > > Is it possible for cdev to be NULL here? > > > > Likewise for other checks for NULL arguments passed to functions > > elsewhere in this patch. > > Hi Simon, > > In the resume path from Sx states it is possible to have a NULL pointer for > the cdev_info pointer. This is due to us not wanting to fail on resuming > unless > absolutely necessary. I went through the rest of the patch looking for NULL > checks > and all of them are valid from my inspection (possible to be NULL). > > Thanks for the review!
Likewise, thanks for checking.
