LKML Archive mirror
 help / color / mirror / Atom feed
From: Vamsi Krishna Attunuru <vattunuru@marvell.com>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: "arnd@arndb.de" <arnd@arndb.de>, Jerin Jacob <jerinj@marvell.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [EXTERNAL] Re: [PATCH v6 1/1] misc: mrvl-cn10k-dpi: add Octeon CN10K DPI administrative driver
Date: Wed, 1 May 2024 07:46:55 +0000	[thread overview]
Message-ID: <MW4PR18MB5244A08DC9D2CF3FAF14314FA6192@MW4PR18MB5244.namprd18.prod.outlook.com> (raw)
In-Reply-To: <2024042950-refurbish-duckbill-77b4@gregkh>



> -----Original Message-----
> From: Greg KH <gregkh@linuxfoundation.org>
> Sent: Monday, April 29, 2024 2:44 PM
> To: Vamsi Krishna Attunuru <vattunuru@marvell.com>
> Cc: arnd@arndb.de; Jerin Jacob <jerinj@marvell.com>; linux-
> kernel@vger.kernel.org
> Subject: [EXTERNAL] Re: [PATCH v6 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 Sun, Apr 28, 2024 at 10:50:27PM -0700, Vamsi Attunuru wrote:
> > Adds a misc driver for Marvell CN10K DPI(DMA Engine) device's physical
> > function which initializes DPI DMA hardware's global configuration and
> > enables hardware mailbox channels between physical function (PF) and
> > it's virtual functions (VF). VF device drivers (User space drivers)
> > use this hw mailbox to communicate any required device configuration
> > on it's respective VF device. Accordingly, this DPI PF driver
> > provisions the VF device resources.
> >
> > At the hardware level, the DPI physical function (PF) acts as a
> > management interface to setup the VF device resources, VF devices are
> > only provisioned to handle or control the actual DMA Engine's data transfer
> capabilities.
> >
> > Signed-off-by: Vamsi Attunuru <vattunuru@marvell.com>
> 
> No one else at Marvell has reviewed this before you submitted this?

It was reviewed until V3, sorry for the inconvenience. I will get the next version
thoroughly reviewed and will send with reviewed-by sign off.
> 
> > ---
> > Changes V5 -> V6:
> > - Updated documentation
> > - Fixed data types in uapi file
> >
> > Changes V4 -> V5:
> > - Fixed license and data types in uapi file
> >
> > Changes V3 -> V4:
> > - Moved ioctl definations to .h file
> > - Fixed structure alignements which are passed in ioctl
> >
> > Changes V2 -> V3:
> > - Added ioctl operation to the fops
> > - Used managed version of kzalloc & request_irq
> > - Addressed miscellaneous comments
> >
> > Changes V1 -> V2:
> > - Fixed return values and busy-wait loops
> > - Merged .h file into .c file
> > - Fixed directory structure
> > - Removed module params
> > - Registered the device as misc device
> >
> >  Documentation/misc-devices/index.rst          |   1 +
> >  Documentation/misc-devices/mrvl_cn10k_dpi.rst |  57 ++
> >  .../userspace-api/ioctl/ioctl-number.rst      |   1 +
> >  MAINTAINERS                                   |   5 +
> >  drivers/misc/Kconfig                          |  13 +
> >  drivers/misc/Makefile                         |   2 +
> >  drivers/misc/mrvl_cn10k_dpi.c                 | 685 ++++++++++++++++++
> >  include/uapi/misc/mrvl_cn10k_dpi.h            |  37 +
> >  8 files changed, 801 insertions(+)
> >
> > diff --git a/Documentation/misc-devices/index.rst
> > b/Documentation/misc-devices/index.rst
> > index 2d0ce9138588..10f2e0f74e45 100644
> > --- a/Documentation/misc-devices/index.rst
> > +++ b/Documentation/misc-devices/index.rst
> > @@ -20,6 +20,7 @@ fit into other categories.
> >     ics932s401
> >     isl29003
> >     lis3lv02d
> > +   mrvl_cn10k_dpi
> >     max6875
> >     oxsemi-tornado
> >     pci-endpoint-test
> 
> Why not in sorted order?
Ack, will fix it.

> 
> > diff --git a/Documentation/misc-devices/mrvl_cn10k_dpi.rst
> > b/Documentation/misc-devices/mrvl_cn10k_dpi.rst
> > new file mode 100644
> > index 000000000000..cce202f114b7
> > --- /dev/null
> > +++ b/Documentation/misc-devices/mrvl_cn10k_dpi.rst
> > @@ -0,0 +1,57 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
> > +===============================================
> > +Marvell CN10K DMA packet interface (DPI) driver
> > +===============================================
> > +
> > +Overview
> > +========
> > +
> > +DPI is a DMA packet interface hardware block in Marvell's CN10K silicon.
> > +DPI hardware comprises a physical function (PF), its virtual
> > +functions, mailbox logic, and a set of DMA engines & DMA command
> queues.
> > +
> > +DPI PF function is an administrative function which services the
> > +mailbox requests from its VF functions and provisions DMA engine
> > +resources to it's VF functions.
> > +
> > +mrvl_cn10k_dpi.ko misc driver loads on DPI PF device and services the
> > +mailbox commands submitted by the VF devices and accordingly
> > +initializes the DMA engines and VF device's DMA command queues. Also,
> > +driver creates /dev/mrvl-cn10k-dpi node to set DMA engine and PEM
> > +(PCIe interface) port attributes like fifo length, molr, mps & mrrs.
> > +
> > +DPI PF driver is just an administrative driver to setup its VF
> > +device's queues and provisions the hardware resources, it can not
> > +initiate any DMA operations. Only VF devices are provisioned with DMA
> capabilities.
> > +
> > +Driver location
> > +===============
> > +
> > +drivers/misc/mrvl_cn10k_dpi.c
> > +
> > +Driver IOCTLs
> > +=============
> > +
> > +:c:macro::`DPI_MPS_MRRS_CFG`
> > +ioctl that sets max payload size & max read request size parameters
> > +of a pem port to which DMA engines are wired.
> > +
> > +
> > +:c:macro::`DPI_ENGINE_CFG`
> > +ioctl that sets DMA engine's fifo sizes & max outstanding load
> > +request thresholds.
> > +
> > +Userspace code example
> > +----------------------
> > +
> > +DPI VF devices are managed by user space drivers, below is a
> > +reference code to the user space driver's mailbox command exchange
> > +with DPI PF driver through hardware mailbox.
> > +
> > +https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__github.com_Vamsi
> > +KrishnaA99_dpi-2Ddma_blob_main_driver_roc-
> 5Fdpi.c&d=DwIBAg&c=nKjWec2b
> >
> +6R0mOyPaz7xtfQ&r=WllrYaumVkxaWjgKto6E_rtDQshhIhik2jkvzFyRhW8&m
> =7PlVgM
> > +7n0EhJ17QxPm3mXq6PT0CQzsOn2YFnRB8RMsIQdFAsZ5UKdTFmkcChUrf-
> &s=8wLYfXsc
> > +lLz7Fuvz9FfyEehC5xTSiIa88PJzturPF4U&e=
> > +
> > +Below is a sample application that uses driver IOCTLs to setup DMA
> > +engine and PEM port attributes over `/dev/mrvl-cn10k-dpi` node.
> > +
> > +https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__github.com_Vamsi
> > +KrishnaA99_dpi-
> 2Ddma_blob_main_application_main.c&d=DwIBAg&c=nKjWec2b
> >
> +6R0mOyPaz7xtfQ&r=WllrYaumVkxaWjgKto6E_rtDQshhIhik2jkvzFyRhW8&m
> =7PlVgM
> > +7n0EhJ17QxPm3mXq6PT0CQzsOn2YFnRB8RMsIQdFAsZ5UKdTFmkcChUrf-
> &s=BMRvLLAx
> > +cnlsyZlVCgOoUXju_jKom8zbKQ9GxlxlApg&e=
> 
> I appreciate the code being open, but this is on a personal account that was
> created for only this one package, and it doesn't even have a README.  Yet it
> is fully owned/copyrighted by Marvell and the code has been around since
> 2021?  Why isn't this on a Marvell-controlled page to show that this really is
> supported and is the proper interface to use this driver that it provides to
> their customers?
>
User space driver is already part of open-source code base(github.com/DPDK/dpdk.git).
The mailbox related changes in user driver (roc_dpi.c file) are not yet upstreamed
which has the dependency with this kernel driver. So, I put the latest user driver
in personal account and shared so that it's helpful for reviewing kernel driver.

I will work internally and get it moved to Marvell-controlled page, will host at
https://github.com/MarvellEmbeddedProcessors and update the same in
Documentation page in next version. 

>
> 
> > diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst
> > b/Documentation/userspace-api/ioctl/ioctl-number.rst
> > index 457e16f06e04..e6fd0c386b59 100644
> > --- a/Documentation/userspace-api/ioctl/ioctl-number.rst
> > +++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
> > @@ -358,6 +358,7 @@ Code  Seq#    Include File
> Comments
> >  0xB6  all    linux/fpga-dfl.h
> >  0xB7  all    uapi/linux/remoteproc_cdev.h                            <mailto:linux-
> remoteproc@vger.kernel.org>
> >  0xB7  all    uapi/linux/nsfs.h                                       <mailto:Andrei Vagin
> <avagin@openvz.org>>
> > +0xB8  01-02  uapi/misc/mrvl_cn10k_dpi.h                              Marvell CN10K DPI
> driver
> >  0xC0  00-0F  linux/usb/iowarrior.h
> >  0xCA  00-0F  uapi/misc/cxl.h
> >  0xCA  10-2F  uapi/misc/ocxl.h
> > diff --git a/MAINTAINERS b/MAINTAINERS index
> > 960512bec428..ab77232d583e 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -13104,6 +13104,11 @@ S:	Supported
> >  F:	Documentation/devicetree/bindings/mmc/marvell,xenon-
> sdhci.yaml
> >  F:	drivers/mmc/host/sdhci-xenon*
> >
> > +MARVELL OCTEON CN10K DPI DRIVER
> > +M:	Vamsi Attunuru <vattunuru@marvell.com>
> > +S:	Maintained
> 
> So this is not part of your job to support this code?  Why isn't Marvell allowing
> that to happen?

No, we are happy to support it further, will fix this.

> 
> > +F:	drivers/misc/mrvl_cn10k_dpi.c
> > +
> >  MATROX FRAMEBUFFER DRIVER
> >  L:	linux-fbdev@vger.kernel.org
> >  S:	Orphan
> > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index
> > 4fb291f0bf7c..78470ef2538f 100644
> > --- a/drivers/misc/Kconfig
> > +++ b/drivers/misc/Kconfig
> > @@ -574,6 +574,19 @@ config NSM
> >  	  To compile this driver as a module, choose M here.
> >  	  The module will be called nsm.
> >
> > +config MARVELL_CN10K_DPI
> > +	tristate "Octeon CN10K DPI driver"
> > +	depends on ARM64 && PCI
> > +	help
> > +	  Enables Octeon CN10K DMA packet interface (DPI) driver which
> intializes
> > +	  DPI hardware's physical function (PF) device's global configuration
> and
> > +	  its virtual function's (VFs) resource configuration to enable DMA
> transfers.
> > +	  DPI PF device does not have any data movement functionality, it
> only serves
> > +	  VF's resource configuration requests.
> 
> Nit, please wrap at 72 columns, didn't checkpatch.pl complain about this?
> 

Yes, I did not see any complains, not sure if I am missing any options. I will
double check and ensure clean patch in next version. 

> > +
> > +	  To compile this driver as a module, choose M here: the module
> > +	  will be called mrvl_cn10k_dpi.
> > +
> >  source "drivers/misc/c2port/Kconfig"
> >  source "drivers/misc/eeprom/Kconfig"
> >  source "drivers/misc/cb710/Kconfig"
> > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index
> > ea6ea5bbbc9c..5106bf96ea5c 100644
> > --- a/drivers/misc/Makefile
> > +++ b/drivers/misc/Makefile
> > @@ -68,3 +68,5 @@ obj-$(CONFIG_TMR_INJECT)	+= xilinx_tmr_inject.o
> >  obj-$(CONFIG_TPS6594_ESM)	+= tps6594-esm.o
> >  obj-$(CONFIG_TPS6594_PFSM)	+= tps6594-pfsm.o
> >  obj-$(CONFIG_NSM)		+= nsm.o
> > +obj-$(CONFIG_MARVELL_CN10K_DPI)	+= mrvl_cn10k_dpi.o
> > +obj-y	+= mrvl_cn10k_dpi.o
> 
> That is odd, why are you saying obj-y here?  You want everyone to always
> build this code into the kernel image no matter what
> 

Sorry, it's my bad, this change(2nd line) was mistakenly added, will remove it.

> > diff --git a/drivers/misc/mrvl_cn10k_dpi.c
> > b/drivers/misc/mrvl_cn10k_dpi.c new file mode 100644 index
> > 000000000000..bd99583994f9
> > --- /dev/null
> > +++ b/drivers/misc/mrvl_cn10k_dpi.c
> > @@ -0,0 +1,685 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Marvell Octeon CN10K DPI driver
> > + *
> > + * Copyright (C) 2024 Marvell.
> > + *
> > + */
> > +
> > +#include <linux/compat.h>
> > +#include <linux/delay.h>
> > +#include <linux/miscdevice.h>
> > +#include <linux/module.h>
> > +#include <linux/pci.h>
> > +#include <linux/irq.h>
> > +#include <linux/interrupt.h>
> > +
> > +#include <uapi/misc/mrvl_cn10k_dpi.h>
> > +
> > +#define DPI_DRV_NAME	"mrvl-cn10k-dpi"
> 
> KBUILD_MODNAME please.

ack
> 
> > +
> > +/* PCI device IDs */
> > +#define PCI_DEVID_MRVL_CN10K_DPI_PF	0xA080
> > +#define PCI_SUBDEVID_MRVL_CN10K_DPI_PF	0xB900
> > +
> > +/* PCI BAR nos */
> 
> What is "nos"?
It's numbers, will add full wording.
> 
> > +#define PCI_DPI_CFG_BAR	0
> > +
> > +/* MSI-X interrupts */
> > +#define DPI_MAX_REQQ_INT 32
> > +#define DPI_MAX_CC_INT   64
> > +
> > +/* MBOX MSI-X interrupt vector index */ #define
> > +DPI_MBOX_PF_VF_INT_IDX 0x75
> > +
> > +#define DPI_MAX_IRQS (DPI_MBOX_PF_VF_INT_IDX + 1)
> > +
> > +#define DPI_MAX_VFS	32
> > +
> > +#define DPI_ENGINE_MASK GENMASK(2, 0)
> > +
> > +#define DPI_DMA_IDS_DMA_NPA_PF_FUNC(x)		(((x) &
> GENMASK_ULL(15, 0)) << 16)
> > +#define DPI_DMA_IDS_INST_STRM(x)		(((x) &
> GENMASK_ULL(7, 0)) << 40)
> > +#define DPI_DMA_IDS_DMA_STRM(x)			(((x) &
> GENMASK_ULL(7, 0)) << 32)
> > +#define DPI_DMA_ENG_EN_MOLR(x)			(((x) &
> GENMASK_ULL(9, 0)) << 32)
> > +#define DPI_EBUS_PORTX_CFG_MPS(x)		(((x) & GENMASK(2,
> 0)) << 4)
> > +#define DPI_DMA_IDS_DMA_SSO_PF_FUNC(x)		((x) &
> GENMASK(15, 0))
> > +#define DPI_DMA_IDS2_INST_AURA(x)		((x) & GENMASK(19,
> 0))
> > +#define DPI_DMA_IBUFF_CSIZE_CSIZE(x)		((x) & GENMASK(13,
> 0))
> > +#define DPI_EBUS_PORTX_CFG_MRRS(x)		((x) & GENMASK(2,
> 0))
> > +#define DPI_ENG_BUF_BLKS(x)			((x) & GENMASK(5,
> 0))
> > +#define DPI_DMA_CONTROL_DMA_ENB
> 	GENMASK_ULL(53, 48)
> > +
> > +#define DPI_DMA_CONTROL_O_MODE			BIT_ULL(14)
> > +#define DPI_DMA_CONTROL_LDWB			BIT_ULL(32)
> > +#define DPI_DMA_CONTROL_WQECSMODE1		BIT_ULL(37)
> > +#define DPI_DMA_CONTROL_ZBWCSEN			BIT_ULL(39)
> > +#define DPI_DMA_CONTROL_WQECSOFF(offset)	(((u64)offset) << 40)
> > +#define DPI_DMA_CONTROL_WQECSDIS		BIT_ULL(47)
> > +#define DPI_DMA_CONTROL_PKT_EN			BIT_ULL(56)
> > +#define DPI_DMA_IBUFF_CSIZE_NPA_FREE		BIT(16)
> > +
> > +#define DPI_CTL_EN				BIT_ULL(0)
> > +#define DPI_DMA_CC_INT				BIT_ULL(0)
> > +#define DPI_DMA_QRST				BIT_ULL(0)
> > +
> > +#define DPI_REQQ_INT_INSTRFLT			BIT_ULL(0)
> > +#define DPI_REQQ_INT_RDFLT			BIT_ULL(1)
> > +#define DPI_REQQ_INT_WRFLT			BIT_ULL(2)
> > +#define DPI_REQQ_INT_CSFLT			BIT_ULL(3)
> > +#define DPI_REQQ_INT_INST_DBO			BIT_ULL(4)
> > +#define DPI_REQQ_INT_INST_ADDR_NULL		BIT_ULL(5)
> > +#define DPI_REQQ_INT_INST_FILL_INVAL		BIT_ULL(6)
> > +#define DPI_REQQ_INT_INSTR_PSN			BIT_ULL(7)
> > +
> > +#define DPI_REQQ_INT \
> > +	(DPI_REQQ_INT_INSTRFLT		| \
> > +	DPI_REQQ_INT_RDFLT		| \
> > +	DPI_REQQ_INT_WRFLT		| \
> > +	DPI_REQQ_INT_CSFLT		| \
> > +	DPI_REQQ_INT_INST_DBO		| \
> > +	DPI_REQQ_INT_INST_ADDR_NULL	| \
> > +	DPI_REQQ_INT_INST_FILL_INVAL	| \
> > +	DPI_REQQ_INT_INSTR_PSN)
> > +
> > +#define DPI_PF_RAS_EBI_DAT_PSN	BIT_ULL(0)
> > +#define DPI_PF_RAS_NCB_DAT_PSN	BIT_ULL(1)
> > +#define DPI_PF_RAS_NCB_CMD_PSN	BIT_ULL(2)
> > +
> > +#define DPI_PF_RAS_INT \
> > +	(DPI_PF_RAS_EBI_DAT_PSN  | \
> > +	 DPI_PF_RAS_NCB_DAT_PSN  | \
> > +	 DPI_PF_RAS_NCB_CMD_PSN)
> > +
> > +#define DPI_DMAX_IBUFF_CSIZE(x)	(0x0ULL | ((x) << 11))
> > +#define DPI_DMAX_IDS(x)		(0x18ULL | ((x) << 11))
> > +#define DPI_DMAX_IDS2(x)	(0x20ULL | ((x) << 11))
> > +#define DPI_DMAX_QRST(x)	(0x30ULL | ((x) << 11))
> > +
> > +#define DPI_CTL		0x10010ULL
> > +#define DPI_DMA_CONTROL 0x10018ULL
> > +#define DPI_DMA_ENGX_EN(x) (0x10040ULL | ((x) << 3))
> > +#define DPI_ENGX_BUF(x)	(0x100C0ULL | ((x) << 3))
> > +
> > +#define DPI_EBUS_PORTX_CFG(x) (0x10100ULL | ((x) << 3))
> > +
> > +#define DPI_PF_RAS 0x10308ULL
> > +#define DPI_PF_RAS_ENA_W1C 0x10318ULL
> > +
> > +#define DPI_DMA_CCX_INT(x) (0x11000ULL | ((x) << 3)) #define
> > +DPI_DMA_CCX_INT_ENA_W1C(x) (0x11800ULL | ((x) << 3))
> > +
> > +#define DPI_REQQX_INT(x) (0x12C00ULL | ((x) << 5)) #define
> > +DPI_REQQX_INT_ENA_W1C(x) (0x13800ULL | ((x) << 5))
> > +
> > +#define DPI_MBOX_PF_VF_DATA0(x) (0x16000ULL | ((x) << 4)) #define
> > +DPI_MBOX_PF_VF_DATA1(x) (0x16008ULL | ((x) << 4))
> > +
> > +#define DPI_MBOX_VF_PF_INT 0x16300ULL #define
> DPI_MBOX_VF_PF_INT_W1S
> > +0x16308ULL #define DPI_MBOX_VF_PF_INT_ENA_W1C 0x16310ULL
> #define
> > +DPI_MBOX_VF_PF_INT_ENA_W1S 0x16318ULL
> > +
> > +#define DPI_WCTL_FIF_THR 0x17008ULL
> > +
> > +#define DPI_EBUS_MAX_PORTS 2
> > +
> > +#define DPI_EBUS_MRRS_MIN 128
> > +#define DPI_EBUS_MRRS_MAX 1024
> > +#define DPI_EBUS_MPS_MIN  128
> > +#define DPI_EBUS_MPS_MAX  1024
> > +#define DPI_WCTL_FIFO_THRESHOLD 0x30
> > +
> > +#define DPI_QUEUE_OPEN  0x1
> > +#define DPI_QUEUE_CLOSE 0x2
> > +#define DPI_REG_DUMP    0x3
> > +#define DPI_GET_REG_CFG 0x4
> > +#define DPI_QUEUE_OPEN_V2 0x5
> > +
> > +enum dpi_mbox_rsp_type {
> > +	DPI_MBOX_TYPE_CMD,
> > +	DPI_MBOX_TYPE_RSP_ACK,
> > +	DPI_MBOX_TYPE_RSP_NACK,
> > +};
> > +
> > +struct dpivf_config {
> > +	u16 csize;
> > +	u32 aura;
> > +	u16 sso_pf_func;
> > +	u16 npa_pf_func;
> 
> Do you intend to have unaligned accesses to the fields in this structure?
> 
No, I will fix it.

> > +};
> > +
> > +struct dpipf_vf {
> > +	u8 this_vfid;
> > +	bool setup_done;
> > +	struct dpivf_config vf_config;
> > +};
> > +
> > +/* DPI device mailbox */
> > +struct dpi_mbox {
> > +	struct work_struct work;
> > +	/* lock to serialize mbox requests */
> > +	struct mutex lock;
> > +	struct dpipf *pf;
> > +	u8 __iomem *pf_vf_data_reg;
> > +	u8 __iomem *vf_pf_data_reg;
> > +};
> > +
> > +struct dpipf {
> > +	struct miscdevice miscdev;
> > +	void __iomem *reg_base;
> > +	struct pci_dev *pdev;
> > +	struct dpipf_vf vf[DPI_MAX_VFS];
> > +	/* Mailbox to talk to VFs */
> > +	struct dpi_mbox *mbox[DPI_MAX_VFS];
> > +};
> > +
> > +union dpi_mbox_message_t {
> 
> Didn't checkpatch complain about the "_t" here?
> 
No, I will double check and fix it.
> > +	u64 u[2];
> 
> What is "u"?
> 
> > +	struct dpi_mbox_message_s {
> > +		/* VF ID to configure */
> > +		u64 vfid           :8;
> > +		/* Command code */
> > +		u64 cmd            :4;
> > +		/* Command buffer size in 8-byte words */
> > +		u64 csize          :14;
> > +		/* 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 */
> > +		u64 rsvd	:42;
> 
> reserved for what?
> 
> If you are reading this from hardware, the bit fields you created here will
> NOT work or be portable at all.  Please do this properly.

Sure, I will fix it properly.
> 
> Also you have a mix of tabs and spaces for some reason in this structure,
> again, checkpatch should have caught that.
> 
> > +	} s __packed;
> 
> "s"?
> 
> What is "u" and "s" here?
> 
> 
> > +};
> > +
> > +static inline void dpi_reg_write(struct dpipf *dpi, u64 offset, u64
> > +val) {
> > +	writeq(val, dpi->reg_base + offset);
> 
> No need to read to ensure that the write succeeded?  Or are you doing that
> in individual places where you want to make sure it happened?  If so, I didn't
> see that in the use of this.

Yes, writes are guaranteed as it's an onboard pcie device and register writes
are uncached memory accesses.  
> 
> > +}
> > +
> > +static inline u64 dpi_reg_read(struct dpipf *dpi, u64 offset) {
> > +	return readq(dpi->reg_base + offset); }
> > +
> > +static void dpi_wqe_cs_offset(struct dpipf *dpi, u8 offset) {
> > +	u64 reg;
> > +
> > +	reg = dpi_reg_read(dpi, DPI_DMA_CONTROL);
> > +	reg &= ~DPI_DMA_CONTROL_WQECSDIS;
> > +	reg |= DPI_DMA_CONTROL_ZBWCSEN |
> DPI_DMA_CONTROL_WQECSMODE1;
> > +	reg |= DPI_DMA_CONTROL_WQECSOFF(offset);
> > +	dpi_reg_write(dpi, DPI_DMA_CONTROL, reg); }
> > +
> > +static int dpi_queue_init(struct dpipf *dpi, struct dpipf_vf *dpivf,
> > +u8 vf) {
> > +	u16 sso_pf_func = dpivf->vf_config.sso_pf_func;
> > +	u16 npa_pf_func = dpivf->vf_config.npa_pf_func;
> > +	u16 csize = dpivf->vf_config.csize;
> > +	u32 aura = dpivf->vf_config.aura;
> > +	unsigned long timeout;
> > +	u64 reg;
> > +
> > +	dpi_reg_write(dpi, DPI_DMAX_QRST(vf), DPI_DMA_QRST);
> > +
> > +	/* Wait for a maximum of 3 sec */
> > +	timeout = jiffies + msecs_to_jiffies(3000);
> > +	while (!time_after(jiffies, timeout)) {
> > +		reg = dpi_reg_read(dpi, DPI_DMAX_QRST(vf));
> > +		if (!(reg & DPI_DMA_QRST))
> > +			break;
> > +
> > +		usleep_range(500, 1000);
> 
> Why sleep this value?  Please document.

ack
> 
> > +	}
> > +
> > +	if (reg & DPI_DMA_QRST) {
> > +		dev_err(&dpi->pdev->dev, "Queue reset failed\n");
> > +		return -EBUSY;
> > +	}
> > +
> > +	dpi_reg_write(dpi, DPI_DMAX_IDS2(vf), 0);
> > +	dpi_reg_write(dpi, DPI_DMAX_IDS(vf), 0);
> > +
> > +	reg = DPI_DMA_IBUFF_CSIZE_CSIZE(csize) |
> DPI_DMA_IBUFF_CSIZE_NPA_FREE;
> > +	dpi_reg_write(dpi, DPI_DMAX_IBUFF_CSIZE(vf), reg);
> > +
> > +	reg = dpi_reg_read(dpi, DPI_DMAX_IDS2(vf));
> > +	reg |= DPI_DMA_IDS2_INST_AURA(aura);
> > +	dpi_reg_write(dpi, DPI_DMAX_IDS2(vf), reg);
> > +
> > +	reg = dpi_reg_read(dpi, DPI_DMAX_IDS(vf));
> > +	reg |= DPI_DMA_IDS_DMA_NPA_PF_FUNC(npa_pf_func);
> > +	reg |= DPI_DMA_IDS_DMA_SSO_PF_FUNC(sso_pf_func);
> > +	reg |= DPI_DMA_IDS_DMA_STRM(vf + 1);
> > +	reg |= DPI_DMA_IDS_INST_STRM(vf + 1);
> > +	dpi_reg_write(dpi, DPI_DMAX_IDS(vf), reg);
> > +
> > +	return 0;
> > +}
> > +
> > +static void dpi_queue_fini(struct dpipf *dpi, struct dpipf_vf *dpivf,
> > +u8 vf) {
> > +	dpi_reg_write(dpi, DPI_DMAX_QRST(vf), DPI_DMA_QRST);
> > +
> > +	/* Reset IDS and IDS2 registers */
> > +	dpi_reg_write(dpi, DPI_DMAX_IDS2(vf), 0);
> > +	dpi_reg_write(dpi, DPI_DMAX_IDS(vf), 0); }
> > +
> > +static void dpi_poll_pfvf_mbox(struct dpipf *dpi) {
> > +	u64 reg;
> > +	u32 vf;
> > +
> > +	reg = dpi_reg_read(dpi, DPI_MBOX_VF_PF_INT);
> > +	if (reg) {
> > +		for (vf = 0; vf < pci_num_vf(dpi->pdev); vf++) {
> > +			if (reg & BIT_ULL(vf))
> > +				schedule_work(&dpi->mbox[vf]->work);
> > +		}
> > +		dpi_reg_write(dpi, DPI_MBOX_VF_PF_INT, reg);
> > +	}
> 
> No error if reg was not read properly?
Yes, ideally no errors as it's an on board pcie device.
> 
> > +}
> > +
> > +static irqreturn_t dpi_mbox_intr_handler(int irq, void *data) {
> > +	struct dpipf *dpi = data;
> > +
> > +	dpi_poll_pfvf_mbox(dpi);
> > +
> > +	return IRQ_HANDLED;
> 
> So this can always succeed?
Yes. It's msix interrupt and not shared, I will check and fix if any thing
need to be validated.

> 
> > +}
> > +
> > +static int queue_config(struct dpipf *dpi, struct dpipf_vf *dpivf,
> > +union dpi_mbox_message_t *msg) {
> > +	int ret = 0;
> > +
> > +	switch (msg->s.cmd) {
> > +	case DPI_QUEUE_OPEN:
> > +	case DPI_QUEUE_OPEN_V2:
> > +		dpivf->vf_config.aura = msg->s.aura;
> > +		dpivf->vf_config.csize = msg->s.cmd == DPI_QUEUE_OPEN ?
> msg->s.csize / 8 :
> > +					 msg->s.csize;
> > +		dpivf->vf_config.sso_pf_func = msg->s.sso_pf_func;
> > +		dpivf->vf_config.npa_pf_func = msg->s.npa_pf_func;
> > +		ret = dpi_queue_init(dpi, dpivf, msg->s.vfid);
> > +		if (!ret) {
> > +			if (msg->s.wqecs)
> > +				dpi_wqe_cs_offset(dpi, msg->s.wqecsoff);
> > +			dpivf->setup_done = true;
> > +		}
> > +		break;
> > +	case DPI_QUEUE_CLOSE:
> > +		dpivf->vf_config.aura = 0;
> > +		dpivf->vf_config.csize = 0;
> > +		dpivf->vf_config.sso_pf_func = 0;
> > +		dpivf->vf_config.npa_pf_func = 0;
> > +		dpi_queue_fini(dpi, dpivf, msg->s.vfid);
> > +		dpivf->setup_done = false;
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +static void dpi_pfvf_mbox_work(struct work_struct *work) {
> > +	struct dpi_mbox *mbox = container_of(work, struct dpi_mbox,
> work);
> > +	union dpi_mbox_message_t msg = { 0 };
> 
> Are you sure this can be on the stack?

No, it's not guaranteed, will use memset. Thanks.
> 
> > +	struct dpipf_vf *dpivf;
> > +	struct dpipf *dpi;
> > +	int ret;
> > +
> > +	dpi = mbox->pf;
> > +
> > +	mutex_lock(&mbox->lock);
> > +	msg.u[0] = readq(mbox->vf_pf_data_reg);
> > +	if (unlikely(msg.u[0] == (u64)-1))
> > +		goto exit;
> 
> Only use likely/unlikely if you can prove with a benchmark that it makes a
> difference.  Otherwise let the CPU and compiler choose for you, as it knows
> better than you do (and will know better over time.)

Ack.
> 
> If you want to use unlikely, you have to document it as to how it matters.
> 
> > +
> > +	if (unlikely(msg.s.vfid >= pci_num_vf(dpi->pdev))) {
> > +		dev_err(&dpi->pdev->dev, "Invalid vfid:%d\n", msg.s.vfid);
> 
> You send this to the kernel log if a device is broken?

Ok, ideally, it's not case, will remove log.

> 
> > +		goto exit;
> > +	}
> > +
> > +	dpivf = &dpi->vf[msg.s.vfid];
> > +	msg.u[1] = readq(mbox->pf_vf_data_reg);
> > +
> > +	ret = queue_config(dpi, dpivf, &msg);
> > +	if (ret < 0)
> > +		writeq(DPI_MBOX_TYPE_RSP_NACK, mbox-
> >pf_vf_data_reg);
> > +	else
> > +		writeq(DPI_MBOX_TYPE_RSP_ACK, mbox->pf_vf_data_reg);
> > +exit:
> > +	mutex_unlock(&mbox->lock);
> > +}
> > +
> > +/* Setup registers for a PF mailbox */ static void
> > +dpi_setup_mbox_regs(struct dpipf *dpi, int vf) {
> > +	struct dpi_mbox *mbox = dpi->mbox[vf];
> > +
> > +	mbox->pf_vf_data_reg = dpi->reg_base +
> DPI_MBOX_PF_VF_DATA0(vf);
> > +	mbox->vf_pf_data_reg = dpi->reg_base +
> DPI_MBOX_PF_VF_DATA1(vf); }
> > +
> > +static int dpi_pfvf_mbox_setup(struct dpipf *dpi) {
> > +	int vf;
> > +
> > +	for (vf = 0; vf < DPI_MAX_VFS; vf++) {
> > +		dpi->mbox[vf] = devm_kzalloc(&dpi->pdev->dev,
> > +sizeof(*dpi->mbox[vf]), GFP_KERNEL);
> > +
> > +		if (!dpi->mbox[vf])
> > +			return -ENOMEM;
> > +
> > +		mutex_init(&dpi->mbox[vf]->lock);
> > +		INIT_WORK(&dpi->mbox[vf]->work, dpi_pfvf_mbox_work);
> > +		dpi->mbox[vf]->pf = dpi;
> > +		dpi_setup_mbox_regs(dpi, vf);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static void dpi_pfvf_mbox_destroy(struct dpipf *dpi) {
> > +	unsigned int vf;
> > +
> > +	for (vf = 0; vf < DPI_MAX_VFS; vf++) {
> > +		if (work_pending(&dpi->mbox[vf]->work))
> > +			cancel_work_sync(&dpi->mbox[vf]->work);
> > +
> > +		dpi->mbox[vf] = NULL;
> > +	}
> > +}
> > +
> > +static void dpi_init(struct dpipf *dpi) {
> > +	unsigned int engine, port;
> > +	u8 mrrs_val, mps_val;
> > +	u64 reg;
> > +
> > +	for (engine = 0; engine < DPI_MAX_ENGINES; engine++) {
> > +		if (engine == 4 || engine == 5)
> > +			reg = DPI_ENG_BUF_BLKS(16);
> > +		else
> > +			reg = DPI_ENG_BUF_BLKS(8);
> > +
> > +		dpi_reg_write(dpi, DPI_ENGX_BUF(engine), reg);
> > +	}
> > +
> > +	reg = DPI_DMA_CONTROL_ZBWCSEN |
> DPI_DMA_CONTROL_PKT_EN | DPI_DMA_CONTROL_LDWB |
> > +	      DPI_DMA_CONTROL_O_MODE |
> DPI_DMA_CONTROL_DMA_ENB;
> > +
> > +	dpi_reg_write(dpi, DPI_DMA_CONTROL, reg);
> > +	dpi_reg_write(dpi, DPI_CTL, DPI_CTL_EN);
> > +
> > +	mrrs_val = 2; /* 512B */
> > +	mps_val = 1; /* 256B */
> > +
> > +	for (port = 0; port < DPI_EBUS_MAX_PORTS; port++) {
> > +		reg = dpi_reg_read(dpi, DPI_EBUS_PORTX_CFG(port));
> > +		reg &= ~(DPI_EBUS_PORTX_CFG_MRRS(7) |
> DPI_EBUS_PORTX_CFG_MPS(7));
> > +		reg |= DPI_EBUS_PORTX_CFG_MPS(mps_val) |
> DPI_EBUS_PORTX_CFG_MRRS(mrrs_val);
> > +		dpi_reg_write(dpi, DPI_EBUS_PORTX_CFG(port), reg);
> > +	}
> > +
> > +	dpi_reg_write(dpi, DPI_WCTL_FIF_THR,
> DPI_WCTL_FIFO_THRESHOLD); }
> > +
> > +static void dpi_fini(struct dpipf *dpi) {
> > +	unsigned int engine;
> > +
> > +	for (engine = 0; engine < DPI_MAX_ENGINES; engine++)
> > +		dpi_reg_write(dpi, DPI_ENGX_BUF(engine), 0);
> > +
> > +	dpi_reg_write(dpi, DPI_DMA_CONTROL, 0);
> > +	dpi_reg_write(dpi, DPI_CTL, 0);
> > +}
> > +
> > +static void dpi_free_irq_vectors(void *pdev) {
> > +	pci_free_irq_vectors((struct pci_dev *)pdev); }
> > +
> > +static int dpi_irq_init(struct dpipf *dpi) {
> > +	struct pci_dev *pdev = dpi->pdev;
> > +	struct device *dev = &pdev->dev;
> > +	int i, ret;
> > +
> > +	/* Clear all RAS interrupts */
> > +	dpi_reg_write(dpi, DPI_PF_RAS, DPI_PF_RAS_INT);
> > +
> > +	/* Clear all RAS interrupt enable bits */
> > +	dpi_reg_write(dpi, DPI_PF_RAS_ENA_W1C, DPI_PF_RAS_INT);
> > +
> > +	for (i = 0; i < DPI_MAX_REQQ_INT; i++) {
> > +		dpi_reg_write(dpi, DPI_REQQX_INT(i), DPI_REQQ_INT);
> > +		dpi_reg_write(dpi, DPI_REQQX_INT_ENA_W1C(i),
> DPI_REQQ_INT);
> > +	}
> > +
> > +	for (i = 0; i < DPI_MAX_CC_INT; i++) {
> > +		dpi_reg_write(dpi, DPI_DMA_CCX_INT(i),
> DPI_DMA_CC_INT);
> > +		dpi_reg_write(dpi, DPI_DMA_CCX_INT_ENA_W1C(i),
> DPI_DMA_CC_INT);
> > +	}
> > +
> > +	ret = pci_alloc_irq_vectors(pdev, DPI_MAX_IRQS, DPI_MAX_IRQS,
> PCI_IRQ_MSIX);
> > +	if (ret != DPI_MAX_IRQS) {
> > +		dev_err(dev, "DPI: Failed to alloc %d msix irqs\n",
> DPI_MAX_IRQS);
> > +		return ret;
> > +	}
> > +
> > +	ret = devm_add_action_or_reset(dev, dpi_free_irq_vectors, pdev);
> > +	if (ret) {
> > +		dev_err(dev, "DPI: Failed to add irq free action\n");
> > +		return ret;
> > +	}
> > +
> > +	ret = devm_request_irq(dev, pci_irq_vector(pdev,
> DPI_MBOX_PF_VF_INT_IDX),
> > +			       dpi_mbox_intr_handler, 0, "dpi-mbox", dpi);
> > +	if (ret) {
> > +		dev_err(dev, "DPI: request_irq failed for mbox; err=%d\n",
> ret);
> > +		return ret;
> > +	}
> > +
> > +	dpi_reg_write(dpi, DPI_MBOX_VF_PF_INT_ENA_W1S,
> GENMASK_ULL(31, 0));
> > +
> > +	return 0;
> > +}
> > +
> > +static int dpi_mps_mrrs_config(struct dpipf *dpi, void __user *arg) {
> > +	struct dpi_mps_mrrs_cfg cfg;
> > +	u8 mrrs_val, mps_val;
> > +	u64 reg;
> > +
> > +	if (copy_from_user(&cfg, arg, sizeof(struct dpi_mps_mrrs_cfg)))
> > +		return -EFAULT;
> > +
> > +	if (cfg.max_read_req_sz < DPI_EBUS_MRRS_MIN ||
> cfg.max_read_req_sz > DPI_EBUS_MRRS_MAX ||
> > +	    !is_power_of_2(cfg.max_read_req_sz)) {
> > +		dev_err(&dpi->pdev->dev, "Invalid MRRS size:%u\n",
> > +cfg.max_read_req_sz);
> 
> You are allowing userspace to spam the kernel log with messages by sending
> the driver invalid data.  THat is a denial of service, please never do that.

Sure, will fix it.
> 
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (cfg.max_payload_sz < DPI_EBUS_MPS_MIN ||
> cfg.max_payload_sz > DPI_EBUS_MPS_MAX ||
> > +	    !is_power_of_2(cfg.max_payload_sz)) {
> > +		dev_err(&dpi->pdev->dev, "Invalid MPS size:%u\n",
> cfg.max_payload_sz);
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (cfg.port >= DPI_EBUS_MAX_PORTS) {
> > +		dev_err(&dpi->pdev->dev, "Invalid EBUS port:%u\n",
> cfg.port);
> > +		return -EINVAL;
> > +	}
> > +
> > +	mrrs_val = fls(cfg.max_read_req_sz >> 8);
> > +	mps_val = fls(cfg.max_payload_sz >> 8);
> > +
> > +	reg = dpi_reg_read(dpi, DPI_EBUS_PORTX_CFG(cfg.port));
> > +	reg &= ~(DPI_EBUS_PORTX_CFG_MRRS(0x7) |
> DPI_EBUS_PORTX_CFG_MPS(0x7));
> > +	reg |= DPI_EBUS_PORTX_CFG_MPS(mps_val) |
> DPI_EBUS_PORTX_CFG_MRRS(mrrs_val);
> > +	dpi_reg_write(dpi, DPI_EBUS_PORTX_CFG(cfg.port), reg);
> > +
> > +	return 0;
> > +}
> > +
> > +static int dpi_engine_config(struct dpipf *dpi, void __user *arg) {
> > +	struct dpi_engine_cfg cfg;
> > +	unsigned int engine;
> > +	u8 *eng_buf;
> > +	u64 reg;
> > +
> > +	if (copy_from_user(&cfg, arg, sizeof(struct dpi_engine_cfg)))
> > +		return -EFAULT;
> 
> No need to ever validate any information in that structure from userspace?
> You always blindly copy it on to the device?  Userspace can never get this
> wrong?
> 
Yes, couple of range checks are needed, will address in next patch.
> > +
> > +	eng_buf = (u8 *)&cfg.fifo_mask;
> > +
> > +	for (engine = 0; engine < DPI_MAX_ENGINES; engine++) {
> > +		reg = DPI_ENG_BUF_BLKS(eng_buf[engine &
> DPI_ENGINE_MASK]);
> > +		dpi_reg_write(dpi, DPI_ENGX_BUF(engine), reg);
> > +
> > +		if (cfg.update_molr) {
> > +			reg = DPI_DMA_ENG_EN_MOLR(cfg.molr[engine &
> DPI_ENGINE_MASK]);
> > +			dpi_reg_write(dpi, DPI_DMA_ENGX_EN(engine),
> reg);
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static long dpi_dev_ioctl(struct file *fptr, unsigned int cmd,
> > +unsigned long data) {
> > +	void __user *arg = (void __user *)data;
> > +	struct dpipf *dpi;
> > +	int ret = -EINVAL;
> 
> Wrong error code for an invalid ioctl command :(
ack
> 
> > +
> > +	dpi = container_of(fptr->private_data, struct dpipf, miscdev);
> > +
> > +	switch (cmd) {
> > +	case DPI_MPS_MRRS_CFG:
> > +		ret = dpi_mps_mrrs_config(dpi, arg);
> > +		break;
> > +	case DPI_ENGINE_CFG:
> > +		ret = dpi_engine_config(dpi, arg);
> > +		break;
> > +	default:
> > +		break;
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +static const struct file_operations dpi_device_fops = {
> > +	.owner = THIS_MODULE,
> > +	.unlocked_ioctl = dpi_dev_ioctl,
> > +	.compat_ioctl = compat_ptr_ioctl,
> > +};
> > +
> > +static int dpi_probe(struct pci_dev *pdev, const struct pci_device_id
> > +*id) {
> > +	struct device *dev = &pdev->dev;
> > +	struct dpipf *dpi;
> > +	int ret;
> > +
> > +	dpi = devm_kzalloc(dev, sizeof(*dpi), GFP_KERNEL);
> > +	if (!dpi)
> > +		return -ENOMEM;
> > +
> > +	dpi->pdev = pdev;
> > +
> > +	ret = pcim_enable_device(pdev);
> > +	if (ret) {
> > +		dev_err(dev, "DPI: Failed to enable PCI device\n");
> > +		return ret;
> > +	}
> > +
> > +	ret = pcim_iomap_regions(pdev, BIT(0) | BIT(4), DPI_DRV_NAME);
> > +	if (ret) {
> > +		dev_err(dev, "DPI: Failed to request MMIO region\n");
> > +		return ret;
> > +	}
> > +
> > +	dpi->reg_base = pcim_iomap_table(pdev)[PCI_DPI_CFG_BAR];
> > +
> > +	/* Initialize global PF registers */
> > +	dpi_init(dpi);
> > +
> > +	/* Setup PF-VF mailbox */
> > +	ret = dpi_pfvf_mbox_setup(dpi);
> > +	if (ret) {
> > +		dev_err(dev, "DPI: Failed to setup pf-vf mbox\n");
> > +		goto err_dpi_fini;
> > +	}
> > +
> > +	/* Register interrupts */
> > +	ret = dpi_irq_init(dpi);
> > +	if (ret) {
> > +		dev_err(dev, "DPI: Failed to initialize irq vectors\n");
> > +		goto err_dpi_mbox_free;
> > +	}
> > +
> > +	pci_set_drvdata(pdev, dpi);
> > +	dpi->miscdev.minor = MISC_DYNAMIC_MINOR;
> > +	dpi->miscdev.name = DPI_DRV_NAME;
> > +	dpi->miscdev.fops = &dpi_device_fops;
> > +	dpi->miscdev.parent = dev;
> > +
> > +	ret = misc_register(&dpi->miscdev);
> > +	if (ret) {
> > +		dev_err(dev, "DPI: Failed to register misc device\n");
> > +		goto err_dpi_mbox_free;
> > +	}
> > +
> > +	return 0;
> > +
> > +err_dpi_mbox_free:
> > +	dpi_pfvf_mbox_destroy(dpi);
> > +err_dpi_fini:
> > +	dpi_fini(dpi);
> > +	return ret;
> > +}
> > +
> > +static void dpi_remove(struct pci_dev *pdev) {
> > +	struct dpipf *dpi = pci_get_drvdata(pdev);
> > +
> > +	misc_deregister(&dpi->miscdev);
> > +	pci_sriov_configure_simple(pdev, 0);
> > +	dpi_pfvf_mbox_destroy(dpi);
> > +	dpi_fini(dpi);
> > +	pci_set_drvdata(pdev, NULL);
> > +}
> > +
> > +static const struct pci_device_id dpi_id_table[] = {
> > +	{ PCI_DEVICE_SUB(PCI_VENDOR_ID_CAVIUM,
> PCI_DEVID_MRVL_CN10K_DPI_PF,
> > +			 PCI_VENDOR_ID_CAVIUM,
> PCI_SUBDEVID_MRVL_CN10K_DPI_PF) },
> > +	{ 0, }  /* end of table */
> > +};
> > +
> > +static struct pci_driver dpi_driver = {
> > +	.name = DPI_DRV_NAME,
> > +	.id_table = dpi_id_table,
> > +	.probe = dpi_probe,
> > +	.remove = dpi_remove,
> > +	.sriov_configure = pci_sriov_configure_simple, };
> > +
> > +module_pci_driver(dpi_driver);
> > +MODULE_DEVICE_TABLE(pci, dpi_id_table);
> MODULE_AUTHOR("Marvell.");
> > +MODULE_DESCRIPTION("Marvell Octeon CN10K DPI Driver");
> > +MODULE_LICENSE("GPL");
> > diff --git a/include/uapi/misc/mrvl_cn10k_dpi.h
> > b/include/uapi/misc/mrvl_cn10k_dpi.h
> > new file mode 100644
> > index 000000000000..a1951644448a
> > --- /dev/null
> > +++ b/include/uapi/misc/mrvl_cn10k_dpi.h
> > @@ -0,0 +1,37 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
> 
> Why is this file "GPL-2.0+" but your driver is "GPL-2.0"?  Is that what your
> lawyers want to have happen (sorry, I have to ask.)

No specific reason, I think I need to use " GPL-2.0-or-later" instead of
"GPL-2.0+", will fix it.

Thanks 
Vamsi

> 
> thanks,
> 
> greg k-h

  reply	other threads:[~2024-05-01  7:47 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-14  3:55 [PATCH 1/1] misc: mrvl-dpi: add octeontx3 dpi driver Vamsi Attunuru
2024-02-14 10:50 ` Greg KH
2024-02-14 11:40   ` [EXT] " Vamsi Krishna Attunuru
2024-02-14 11:22 ` Arnd Bergmann
2024-02-14 13:33   ` [EXT] " Vamsi Krishna Attunuru
2024-02-16 10:32     ` [PATCH v2 1/1] misc: mrvl-cn10k-dpi: add Octeon CN10K " Vamsi Attunuru
2024-02-17  8:13       ` Greg KH
2024-02-19  5:03         ` [EXT] " Vamsi Krishna Attunuru
2024-02-19  6:18           ` Greg KH
2024-02-19  7:03             ` Vamsi Krishna Attunuru
2024-02-28 16:21             ` [PATCH v3 1/1] misc: mrvl-cn10k-dpi: add Octeon CN10K DPI administrative driver Vamsi Attunuru
2024-03-07 21:55               ` Greg KH
2024-03-08 11:36                 ` [EXTERNAL] " Vamsi Krishna Attunuru
2024-03-12 10:56                   ` [PATCH v4 " Vamsi Attunuru
2024-04-11 13:02                     ` Greg KH
2024-04-12  6:34                       ` [EXTERNAL] " Vamsi Krishna Attunuru
2024-04-12 12:10                         ` [PATCH v5 " Vamsi Attunuru
2024-04-12 12:26                           ` Greg KH
2024-04-12 13:56                             ` [EXTERNAL] " Vamsi Krishna Attunuru
2024-04-12 15:34                               ` Greg KH
2024-04-12 16:19                                 ` Vamsi Krishna Attunuru
2024-04-13  5:47                                   ` Greg KH
2024-04-13 10:58                                     ` Vamsi Krishna Attunuru
2024-04-13 11:25                                       ` Greg KH
2024-04-13 16:17                                         ` Vamsi Krishna Attunuru
2024-04-13 19:11                                           ` Arnd Bergmann
2024-04-14  9:33                                             ` Vamsi Krishna Attunuru
2024-04-14  9:46                                               ` Greg Kroah-Hartman
2024-04-14 12:32                                                 ` Vamsi Krishna Attunuru
2024-04-25 13:36                               ` Vamsi Krishna Attunuru
2024-04-26  1:04                                 ` Greg KH
2024-04-26 18:20                                   ` [PATCH v6 " Vamsi Attunuru
2024-04-27 11:06                                     ` Greg KH
2024-04-27 11:59                                       ` [EXTERNAL] " Vamsi Krishna Attunuru
2024-04-29  5:50                                         ` Vamsi Attunuru
2024-04-29  9:13                                           ` Greg KH
2024-05-01  7:46                                             ` Vamsi Krishna Attunuru [this message]
2024-05-01  8:02                                               ` [EXTERNAL] " Greg KH
2024-05-20 11:06                                             ` [PATCH v7 " Vamsi Attunuru
2024-04-30 14:00                                           ` [PATCH v6 " kernel test robot
2024-04-30 14:22                                           ` kernel test robot
2024-04-30 15:36                                           ` kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=MW4PR18MB5244A08DC9D2CF3FAF14314FA6192@MW4PR18MB5244.namprd18.prod.outlook.com \
    --to=vattunuru@marvell.com \
    --cc=arnd@arndb.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=jerinj@marvell.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).