Date:   Thu, 16 Feb 2023 12:24:12 +0100
From:   Greg KH <gregkh@linuxfoundation.org>
To:     Pawel Laszczak <pawell@cadence.com>
Cc:     linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
        Daisy.Barrera@siriusxm.com, Cliff.Holden@siriusxm.com,
        arnd@arndb.de, tony@atomide.com, jdelvare@suse.de,
        neal_liu@aspeedtech.com, linus.walleij@linaro.org,
        egtvedt@samfundet.no, biju.das.jz@bp.renesas.com,
        herve.codina@bootlin.com
Subject: Re: [PATCH 1/4] usb: cdns2: Device side header file for CDNS2 driver
Message-ID: <Y+4SXE25Xl7Lv/K8@kroah.com>
References: <20230216105411.364157-1-pawell@cadence.com>
 <20230216105411.364157-2-pawell@cadence.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230216105411.364157-2-pawell@cadence.com>
Precedence: bulk

On Thu, Feb 16, 2023 at 05:54:08AM -0500, Pawel Laszczak wrote:
> Patch defines macros, registers and structures used by
> Device side driver.
> 
> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> ---
>  drivers/usb/gadget/udc/cdns2/cdns2-gadget.h | 670 ++++++++++++++++++++
>  1 file changed, 670 insertions(+)
>  create mode 100644 drivers/usb/gadget/udc/cdns2/cdns2-gadget.h
> 
> diff --git a/drivers/usb/gadget/udc/cdns2/cdns2-gadget.h b/drivers/usb/gadget/udc/cdns2/cdns2-gadget.h
> new file mode 100644
> index 000000000000..66cdb902163c
> --- /dev/null
> +++ b/drivers/usb/gadget/udc/cdns2/cdns2-gadget.h
> @@ -0,0 +1,670 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * USBHS-DEV device controller driver header file
> + *
> + * Copyright (C) 2023 Cadence.
> + *
> + * Author: Pawel Laszczak <pawell@cadence.com>
> + */
> +
> +#ifndef __LINUX_CDNS2_GADGET
> +#define __LINUX_CDNS2_GADGET
> +
> +#include <linux/usb/gadget.h>
> +#include <linux/dma-direction.h>
> +
> +/*
> + * USBHS register interface.
> + * This corresponds to the USBHS Device Controller Interface.
> + */
> +
> +/**
> + * struct cdns2_ep0_regs - endpoint 0 related registers.
> + * @rxbc: receive (OUT) 0 endpoint byte count register.
> + * @txbc: transmit (IN) 0 endpoint byte count register.
> + * @cs: 0 endpoint control and status register.
> + * @fifo: 0 endpoint fifo register.
> + * @setupdat: SETUP data register.
> + * @maxpack: 0 endpoint max packet size.

You didn't describe the "reserved" fields in here, so you will get a
kerneldoc warning :(

> + */
> +struct cdns2_ep0_regs {
> +	__u8 rxbc;
> +	__u8 txbc;
> +	__u8 cs;
> +	__u8 reserved1[4];
> +	__u8 fifo;
> +	__le32 reserved2[94];
> +	__u8 setupdat[8];
> +	__u8 reserved4[88];

Why are these all reserved?  Should they be set to 0?  Something else?

Same with the other reserved fields in this file, you need to fix up the
kerneldoc for them too.

thanks,

greg k-h