Received-SPF: pass (google.com: domain of linux-kernel+bounces-201015-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Tue, 4 Jun 2024 17:52:46 +0200
From: Greg KH <gregkh@linuxfoundation.org>
To: Vamsi Attunuru <vattunuru@marvell.com>
Cc: jerinj@marvell.com, schalla@marvell.com, arnd@arndb.de,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v7 1/1] misc: mrvl-cn10k-dpi: add Octeon CN10K DPI
 administrative driver
Message-ID: <2024060412-amulet-unflawed-f37a@gregkh>
References: <2024042950-refurbish-duckbill-77b4@gregkh>
 <20240520110630.2568767-1-vattunuru@marvell.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240520110630.2568767-1-vattunuru@marvell.com>

On Mon, May 20, 2024 at 04:06:30AM -0700, Vamsi Attunuru wrote:
> +union dpi_mbox_message {
> +	u64 word[2];
> +	struct {
> +#if defined(__BIG_ENDIAN_BITFIELD)
> +		/* SSO PF function */
> +		u64 sso_pf_func  :16;
> +		/* Aura of the command buffer */
> +		u64 aura         :20;
> +		/* Command buffer size in 8-byte words */
> +		u64 csize        :16;
> +		/* Command code */
> +		u64 cmd          :4;
> +		/* VF ID to configure */
> +		u64 vfid         :8;
> +		/* Reserved for future use */
> +		u64 rsvd_85_127  :40;
> +		/* Work queue completion status byte offset */
> +		u64 wqecsoff     :7;
> +		/* Work queue completion status enable */
> +		u64 wqecs        :1;
> +		/* NPA PF function */
> +		u64 npa_pf_func  :16;
> +#else
> +		/* VF ID to configure */
> +		u64 vfid         :8;
> +		/* Command code */
> +		u64 cmd          :4;
> +		/* Command buffer size in 8-byte words */
> +		u64 csize        :16;
> +		/* Aura of the command buffer */
> +		u64 aura         :20;
> +		/* SSO PF function */
> +		u64 sso_pf_func  :16;
> +		/* NPA PF function */
> +		u64 npa_pf_func  :16;
> +		/* Work queue completion status enable */
> +		u64 wqecs        :1;
> +		/* Work queue completion status byte offset */
> +		u64 wqecsoff     :7;
> +		/* Reserved for future use */
> +		u64 rsvd_85_127  :40;
> +#endif
> +	};
> +};

The ifdef is cute, but not correct, sorry.  Please use bit shifts to
handle this properly without any #ifdef needed at all.


> +
> +static inline void dpi_reg_write(struct dpipf *dpi, u64 offset, u64 val)
> +{
> +	writeq(val, dpi->reg_base + offset);

No read needed after a write to ensure the write made it to the hardware
properly?

thanks,

greg k-h