RE: [PATCH v7 5/6] usb:cdns3 Add Cadence USB3 DRD Driver

From: Pawel Laszczak <pawell@cadence.com>
To: Roger Quadros <rogerq@ti.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>
Cc: "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"felipe.balbi@linux.intel.com" <felipe.balbi@linux.intel.com>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	"hdegoede@redhat.com" <hdegoede@redhat.com>,
	"heikki.krogerus@linux.intel.com"
	<heikki.krogerus@linux.intel.com>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"jbergsagel@ti.com" <jbergsagel@ti.com>,
	"nsekhar@ti.com" <nsekhar@ti.com>, "nm@ti.com" <nm@ti.com>,
	Suresh Punnoose <sureshp@cadence.com>,
	"peter.chen@nxp.com" <peter.chen@nxp.com>,
	Jayshri Dajiram Pawar <jpawar@cadence.com>,
	Rahul Kumar <kurahul@cadence.com>
Subject: RE: [PATCH v7 5/6] usb:cdns3 Add Cadence USB3 DRD Driver
Date: Fri, 7 Jun 2019 07:59:38 +0000	[thread overview]
Message-ID: <BYAPR07MB4709E1C8B190E5BBC055DC34DD100@BYAPR07MB4709.namprd07.prod.outlook.com> (raw)
In-Reply-To: <a6eb0f66-14a7-d235-94a4-a53d291ac2c3@ti.com>

>
>On 05/06/2019 13:03, Pawel Laszczak wrote:
>> This patch introduce new Cadence USBSS DRD driver to Linux kernel.
>>
>> The Cadence USBSS DRD Driver is a highly configurable IP Core which
>> can be instantiated as Dual-Role Device (DRD), Peripheral Only and
>> Host Only (XHCI)configurations.
>>
>> The current driver has been validated with FPGA platform. We have
>> support for PCIe bus, which is used on FPGA prototyping.
>>
>> The host side of USBSS-DRD controller is compliant with XHCI
>> specification, so it works with standard XHCI Linux driver.
>>
>> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
>> ---
>>  drivers/usb/Kconfig                |    2 +
>>  drivers/usb/Makefile               |    2 +
>>  drivers/usb/cdns3/Kconfig          |   44 +
>>  drivers/usb/cdns3/Makefile         |   14 +
>>  drivers/usb/cdns3/cdns3-pci-wrap.c |  157 ++
>>  drivers/usb/cdns3/core.c           |  529 +++++++
>>  drivers/usb/cdns3/core.h           |  121 ++
>>  drivers/usb/cdns3/debug.h          |  173 +++
>>  drivers/usb/cdns3/debugfs.c        |  173 +++
>>  drivers/usb/cdns3/drd.c            |  379 +++++
>>  drivers/usb/cdns3/drd.h            |  166 ++
>>  drivers/usb/cdns3/ep0.c            |  915 +++++++++++
>>  drivers/usb/cdns3/gadget-export.h  |   28 +
>>  drivers/usb/cdns3/gadget.c         | 2290 ++++++++++++++++++++++++++++
>>  drivers/usb/cdns3/gadget.h         | 1313 ++++++++++++++++
>>  drivers/usb/cdns3/host-export.h    |   28 +
>>  drivers/usb/cdns3/host.c           |   76 +
>>  drivers/usb/cdns3/trace.c          |   23 +
>>  drivers/usb/cdns3/trace.h          |  447 ++++++
>>  19 files changed, 6880 insertions(+)
>>  create mode 100644 drivers/usb/cdns3/Kconfig
>>  create mode 100644 drivers/usb/cdns3/Makefile
>>  create mode 100644 drivers/usb/cdns3/cdns3-pci-wrap.c
>>  create mode 100644 drivers/usb/cdns3/core.c
>>  create mode 100644 drivers/usb/cdns3/core.h
>>  create mode 100644 drivers/usb/cdns3/debug.h
>>  create mode 100644 drivers/usb/cdns3/debugfs.c
>>  create mode 100644 drivers/usb/cdns3/drd.c
>>  create mode 100644 drivers/usb/cdns3/drd.h
>>  create mode 100644 drivers/usb/cdns3/ep0.c
>>  create mode 100644 drivers/usb/cdns3/gadget-export.h
>>  create mode 100644 drivers/usb/cdns3/gadget.c
>>  create mode 100644 drivers/usb/cdns3/gadget.h
>>  create mode 100644 drivers/usb/cdns3/host-export.h
>>  create mode 100644 drivers/usb/cdns3/host.c
>>  create mode 100644 drivers/usb/cdns3/trace.c
>>  create mode 100644 drivers/usb/cdns3/trace.h
>>
>
><snip>
>
>
>> +/**
>> + * cdns3_probe - probe for cdns3 core device
>> + * @pdev: Pointer to cdns3 core platform device
>> + *
>> + * Returns 0 on success otherwise negative errno
>> + */
>> +static int cdns3_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct resource	*res;
>> +	struct cdns3 *cdns;
>> +	void __iomem *regs;
>> +	int ret;
>> +
>> +	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
>
>Are all cores supported by this driver limited to 32-bit? If not then
>you need to set the mask based on compatibles?

Two version: 0x00024509 and 0x0002450d support extended DMA address to 48 bits.
The problem is that the 16 most significant bit indicate the segment that can be set 
per endpoint.  We can assume that it will be constant per endpoint. All DMA memory
for data buffers and for TRBs must be in this range.  

I think that Linux kernel (DMA subsystem) can't guarantee it. 

Using dma_set_mask_and_coherent(dev, DMA_BIT_MASK(48)) not guarantee that segment 
part will be always the same.  

So the safest way is to limit DMA range to 32 bits.  

><snip>

Regards,
Pawel