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

[Vamsi Krishna Attunuru]

> -----Original Message-----
> From: Vamsi Krishna Attunuru
> Sent: Tuesday, June 4, 2024 9:51 PM
> To: 'Greg KH' <gregkh@xxxxxxxxxxxxxxxxxxx>
> Cc: Jerin Jacob <jerinj@xxxxxxxxxxx>; Srujana Challa <schalla@xxxxxxxxxxx>;
> arnd@xxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: RE: [EXTERNAL] Re: [PATCH v7 1/1] misc: mrvl-cn10k-dpi: add Octeon
> CN10K DPI administrative driver
> 
> 
> 
> > -----Original Message-----
> > From: Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx>
> > Sent: Tuesday, June 4, 2024 9:23 PM
> > To: Vamsi Krishna Attunuru <vattunuru@xxxxxxxxxxx>
> > Cc: Jerin Jacob <jerinj@xxxxxxxxxxx>; Srujana Challa
> > <schalla@xxxxxxxxxxx>; arnd@xxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> > Subject: [EXTERNAL] Re: [PATCH v7 1/1] misc: mrvl-cn10k-dpi: add
> > Octeon CN10K DPI administrative driver
> >
> > Prioritize security for external emails: Confirm sender and content
> > safety before clicking links or opening attachments
> >
> > ----------------------------------------------------------------------
> > 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.
> >
> Ack, will fix it next version. Thanks for the suggestion.
> 

Hi Greg, the ARM64 cores on the Octeon CN10K hardware platform always run in LE mode and this CN10K DPI PF driver is only supported on Octeon CN10K platforms as the DPI PF device is an onboard PCIe device. Can I remove the BE format and only define the LE format for the dpi_mbox_message structure?, other HW device drivers of Octeon CN10K platform also only support LE format.

Regards
Vamsi

> >
> >
> >
> > > +
> > > +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?
> 
> Yes, as it's an onboard PCIe device, writes will happen properly. I will modify
> it as write followed by a write barrier to avoid any reordering.
> 
> Thanks
> Vamsi
> >
> > thanks,
> >
> > greg k-h