From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id CCE222E9EBB for ; Thu, 29 Jan 2026 13:32:54 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1769693574; cv=none; b=thN1PnbgnwtWE8lNL5veYuyNjrSCQrdPRLCDtU4oU1Qw89WvZ8GiqXgHsqdRB7xbMkX8Xf2hDLfk+8EkM3hPONzxSRL6ID3qrCQmVCqM2GDEUZ8f6J0f0m+J6hr1j6/l9+WCfiYJ/uIWjoHjDmOXhjz5h3Z9ra2SP+D7ILESrjA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1769693574; c=relaxed/simple; bh=uLSaw73Lzo5Zt8kvi8BOTLQB68iPZBmqiBEuojieE4E=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=BQRbSFT2SFCfibuG7/naRGp5r07Dlm7y1+K+s2jZBkEMCIEeZ9R//RrZv+wClZj3/v2JTtq9Vr1VjrwB9IxIIQOW7I7qJcUyHhz7SH/p40ko9s4uUPIX8sga40WrLTpP1q++dQ3n2SCPQN/d3k6o9A3nAwO/o55u6oeLqfj+hoA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=kWZ6/RT0; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="kWZ6/RT0" Received: by smtp.kernel.org (Postfix) with ESMTPSA id BCD6CC4CEF7; Thu, 29 Jan 2026 13:32:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1769693574; bh=uLSaw73Lzo5Zt8kvi8BOTLQB68iPZBmqiBEuojieE4E=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=kWZ6/RT0yvXleqrZHQrv4pdtyIDyBG9acGMGR5/ku/TVMKKVQk2lvidpCogb1y0Ga 8duvUwUyQtMm7tdwqTB3sz5UIgHwYPUoeYwIBThb+k8apLaB/c5V5p8rvYeymoXmP8 iHmnSxhyylvm31MAwlZocxvuMCn7tgVg4HggnHA/2qE0YPar18AYLZU6MVNUQEt0iy dAASIhuGPxXz8AyNuWPf+UlYeli8M+i53EVi2FKaS66VPCFkbRitmujzqflALB0FSY tj7v+CoeH6CRwec/NNVAYOWeSiBVuPlTHm7TzGQ71eKZh+YyX/9mA8TjeW06KtOs3I cxwTrotEOMqCg== Date: Thu, 29 Jan 2026 15:32:48 +0200 From: Leon Romanovsky To: Pavan Chebbi Cc: jgg@ziepe.ca, michael.chan@broadcom.com, linux-kernel@vger.kernel.org, dave.jiang@intel.com, saeedm@nvidia.com, Jonathan.Cameron@huawei.com, gospo@broadcom.com, selvin.xavier@broadcom.com, kalesh-anakkur.purayil@broadcom.com Subject: Re: [PATCH v2 fwctl 2/5] fwctl/bnxt_en: Refactor aux bus functions to be more generic Message-ID: <20260129133248.GJ10992@unreal> References: <20260126053710.3474483-1-pavan.chebbi@broadcom.com> <20260126053710.3474483-3-pavan.chebbi@broadcom.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20260126053710.3474483-3-pavan.chebbi@broadcom.com> On Sun, Jan 25, 2026 at 09:37:07PM -0800, Pavan Chebbi wrote: > Up until now there was only one auxiliary device that bnxt > created and that was for RoCE driver. bnxt fwctl is also > going to use an aux bus device that bnxt should create. > This requires some nomenclature changes and refactoring of > the existing bnxt aux dev functions. > > Convert 'aux_priv' and 'edev' members of struct bnxt into > arrays where each element contains supported auxbus device's > data. Move struct bnxt_aux_priv from bnxt.h to ulp.h because > that is where it belongs. Make aux bus init/uninit/add/del > functions more generic which will loop through all the aux > device types. Make bnxt_ulp_start/stop functions (the only > other common functions applicable to any aux device) loop > through the aux devices to update their config and states. > Make callers of bnxt_ulp_start() call it only when there > are no errors. > > Also, as an improvement in code, bnxt_register_dev() can skip > unnecessary dereferencing of edev from bp, instead use the > edev pointer from the function parameter. > > Future patches will reuse these functions to add an aux bus > device for fwctl. > > Reviewed-by: Andy Gospodarek > Signed-off-by: Pavan Chebbi > --- > drivers/net/ethernet/broadcom/bnxt/bnxt.c | 47 ++- > drivers/net/ethernet/broadcom/bnxt/bnxt.h | 19 +- > .../net/ethernet/broadcom/bnxt/bnxt_devlink.c | 8 +- > .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 2 +- > drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 334 +++++++++++------- > include/linux/bnxt/ulp.h | 25 +- > 6 files changed, 271 insertions(+), 164 deletions(-) <...> > +static void bnxt_auxdev_set_state(struct bnxt *bp, int idx, int state) > +{ > + mutex_lock(&bp->auxdev_lock); > + bp->auxdev_state[idx] = state; > + mutex_unlock(&bp->auxdev_lock); > +} > + > +static bool bnxt_auxdev_is_init(struct bnxt *bp, int idx) > +{ > + bool ret; > + > + mutex_lock(&bp->auxdev_lock); > + ret = (bp->auxdev_state[idx] == BNXT_ADEV_STATE_INIT); > + mutex_unlock(&bp->auxdev_lock); > + return ret; > +} > + > +static bool bnxt_auxdev_is_active(struct bnxt *bp, int idx) > +{ > + bool ret; > + > + mutex_lock(&bp->auxdev_lock); > + ret = (bp->auxdev_state[idx] == BNXT_ADEV_STATE_ADD); > + mutex_unlock(&bp->auxdev_lock); > + return ret; > +} <...> > void bnxt_ulp_stop(struct bnxt *bp) > { > - struct bnxt_aux_priv *aux_priv = bp->aux_priv; > - struct bnxt_en_dev *edev = bp->edev; > - > - if (!edev) > - return; > + int i; > > - mutex_lock(&edev->en_dev_lock); > - if (!bnxt_ulp_registered(edev) || > - (edev->flags & BNXT_EN_FLAG_ULP_STOPPED)) > - goto ulp_stop_exit; > - > - edev->flags |= BNXT_EN_FLAG_ULP_STOPPED; > - if (aux_priv) { > + for (i = 0; i < __BNXT_AUXDEV_MAX; i++) { > + struct bnxt_aux_priv *aux_priv; > struct auxiliary_device *adev; > + struct bnxt_en_dev *edev; > + > + if (!bnxt_auxdev_is_active(bp, i)) It’s good that you stopped using atomic_t for state variables, but the locking is still incorrect. It’s not enough to hold the lock only while reading the state; you need to keep the lock for the entire period during which the auxdev is expected to remain active. Thanks