RE: [PATCH v2 06/10] usb: cdnsp: Device side header file for CDNSP driver

[Pawel Laszczak <pawell@cadence.com>]

Hi,

>On 20-11-06 12:42:56, Pawel Laszczak wrote:
>> Patch defines macros, registers and structures used by
>> Device side driver.
>>
>> Because the size of main patch is very big, I’ve decided to create
>> separate patch for cdnsp-gadget.h. It should simplify reviewing the code.
>>
>> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
>> ---
>>  drivers/usb/cdns3/cdnsp-gadget.h | 1463 ++++++++++++++++++++++++++++++
>>  1 file changed, 1463 insertions(+)
>>  create mode 100644 drivers/usb/cdns3/cdnsp-gadget.h
>>
>> diff --git a/drivers/usb/cdns3/cdnsp-gadget.h b/drivers/usb/cdns3/cdnsp-gadget.h
>> new file mode 100644
>> index 000000000000..29d5e2d00879
>> --- /dev/null
>> +++ b/drivers/usb/cdns3/cdnsp-gadget.h
>> @@ -0,0 +1,1463 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Cadence CDNSP DRD Driver.
>> + *
>> + * Copyright (C) 2020 Cadence.
>> + *
>> + * Author: Pawel Laszczak <pawell@cadence.com>
>> + *
>> + * Code based on Linux XHCI driver.
>> + * Origin: Copyright (C) 2008 Intel Corp.
>> + */
>> +#ifndef __LINUX_CDNSP_GADGET_H
>> +#define __LINUX_CDNSP_GADGET_H
>> +
>> +#include <linux/io-64-nonatomic-lo-hi.h>
>> +#include <linux/usb/gadget.h>
>> +#include <linux/irq.h>
>> +
>> +/* Max number slots - only 1 is allowed. */
>> +#define CDNSP_DEV_MAX_SLOTS	1
>> +
>> +#define CDNSP_EP0_SETUP_SIZE	512
>> +
>> +/*16 for in and 16 for out. */
>> +#define CDNSP_ENDPOINTS_NUM	32
>> +
>> +/* Best Effort Service Latency. */
>> +#define CDNSP_DEFAULT_BESL	0
>> +
>> +/* Device Controller command default timeout value in us */
>> +#define CDNSP_CMD_TIMEOUT	(15 * 1000)
>> +
>> +/* Up to 16 ms to halt an device controller */
>> +#define CDNSP_MAX_HALT_USEC	(16 * 1000)
>> +
>> +#define CDNSP_CTX_SIZE	2112
>> +
>> +/*
>> + * Controller register interface.
>> + */
>> +
>> +/**
>> + * struct cdnsp_cap_regs - CDNSP Registers.
>> + * @hc_capbase:	Length of the capabilities register and controller
>> + *              version number
>> + * @hcs_params1: HCSPARAMS1 - Structural Parameters 1
>> + * @hcs_params2: HCSPARAMS2 - Structural Parameters 2
>> + * @hcs_params3: HCSPARAMS3 - Structural Parameters 3
>> + * @hcc_params: HCCPARAMS - Capability Parameters
>> + * @db_off: DBOFF - Doorbell array offset
>> + * @run_regs_off: RTSOFF - Runtime register space offset
>> + * @hcc_params2: HCCPARAMS2 Capability Parameters 2,
>> + */
>> +struct cdnsp_cap_regs {
>> +	__le32 hc_capbase;
>> +	__le32 hcs_params1;
>> +	__le32 hcs_params2;
>> +	__le32 hcs_params3;
>> +	__le32 hcc_params;
>> +	__le32 db_off;
>> +	__le32 run_regs_off;
>> +	__le32 hcc_params2;
>> +	/* Reserved up to (CAPLENGTH - 0x1C) */
>> +};
>> +
>> +/* hc_capbase bitmasks. */
>> +/* bits 7:0 - how long is the Capabilities register. */
>> +#define HC_LENGTH(p)		(((p) >> 00) & GENMASK(7, 0))
>> +/* bits 31:16	*/
>> +#define HC_VERSION(p)		(((p) >> 16) & GENMASK(15, 1))
>> +
>> +/* HCSPARAMS1 - hcs_params1 - bitmasks */
>> +/* bits 0:7, Max Device Endpoints */
>> +#define HCS_ENDPOINTS_MASK	GENMASK(7, 0)
>> +#define HCS_ENDPOINTS(p)	(((p) & HCS_ENDPOINTS_MASK) >> 0)
>> +
>> +/* HCCPARAMS offset from PCI base address */
>> +#define HCC_PARAMS_OFFSET	0x10
>> +
>> +/* HCCPARAMS - hcc_params - bitmasks */
>> +/* true: device controller can use 64-bit address pointers. */
>
>What does "true" mean at above comment?

I understand that it's ambiguous. 

True is not always equal  1. 
I will replace it with 1. 

Thanks for review.

<snip>  

--
Regards,
Pawel