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

["Arnd Bergmann" <arnd@arndb.de>]

On Tue, Jun 18, 2024, at 15:09, Vamsi Attunuru wrote:
>Changes V7 -> V8:
> - Used bit shift operations to access mbox msg fields
> - Removed bitfields in mailbox msg structure

Thanks for the changes, looks good to me. Two more things:

> +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);
> +	}
> +}
> +
> +static irqreturn_t dpi_mbox_intr_handler(int irq, void *data)
> +{
> +	struct dpipf *dpi = data;
> +
> +	dpi_poll_pfvf_mbox(dpi);
> +
> +	return IRQ_HANDLED;
> +}

[minor cleanup]
The second function doesn't do much here, so you could
just merge them into one. Or to simplify it further, you
could just use request_threaded_irq() and avoid the separate
workqueue as well.

> +struct dpi_mps_mrrs_cfg {
> +	__u16 max_read_req_sz; /* Max read request size */
> +	__u16 max_payload_sz;  /* Max payload size */
> +	__u8 port; /* Ebus port */
> +};
> +
> +struct dpi_engine_cfg {
> +	__u64 fifo_mask; /* FIFO size mask in KBytes */
> +	__u16 molr[DPI_MAX_ENGINES]; /* Max outstanding load requests */
> +	__u8 update_molr; /* '1' to update engine MOLR */
> +};

Both of these structures have architecture specific padding
at the end because the members don't add up to a multiple
of words. Please add explicit reserved fields to a multiple of
8 bytes for each one, or make the fields longer.

Documentation/driver-api/ioctl.rst has more details on this.

     Arnd