Re: [PATCH v7 1/1] misc: mrvl-cn10k-dpi: add Octeon CN10K DPI administrative driver

[Greg KH <gregkh@linuxfoundation.org>]

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