Re: [PATCH 00/31] Introduced new Cadence USBSSP DRD Driver

From: Roger Quadros
Date: Thu Aug 02 2018 - 12:04:33 EST


On 02/08/18 07:26, Pawel Laszczak wrote:
>>> This patch set introduce new Cadence USBSSP DRD driver to linux
>>> kernel.
>>>
>>> The Cadence USBSSP DRD Driver s 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 burned. We have
>>> support for PCIe bus, which is used on FPGA prototyping.
>>>
>>> The host site of USBSSP controller is compliance with XHCI
>>> specification, so it works with standard XHCI linux driver.
>>>
>>> Also, device side of USBSSP controller was designed in such way to
>>> looks like XHCI. It means that most of logic of USBSSP controller is
>>> also compliance with XHCI specification.
>>>
>>> Consequently, the USBSSP driver for peripheral mode is very similar to
>>> XHCI driver.
>>>
>>> This version of driver supports only Device mode but DRD and Host mode
>>> will be added in the future.
>>>
>>
>> Based on the posting date this series looks like v3.
>>
>> You should add the version prefix to the patches next time.
>> e.g. [PATCH v4 ...]
>>
> I wanted to start versioning driver after this set patch will be approved.
> Also I have plan to add change-log after approval this patch set.
> I now treat this patch set as Initial version of driver.
> If my approach is incorrect please correct me ??
> Maybe before approval the better versioning will be [PATCH vrc4] ?

You should prefix patch with RFC.

[RFC PATCH v4]

Drop the RFC and reset version to 1 when things are ready for integration.

>
>> It is also recommended to add a Change-log in the cover letter explaining
>> what changes were made in each revision.
>>
> I
>
>> There are a bunch of checkpatch warnings in this series.
>> You can use ./scripts/checkpatch.pl on the patches and fix the issues.
> Yes I know, but most of them are related to "line over 80 characters"
> and " quoted string split across lines" warnings. The first group I could correct
> but then I will make readability worse.
> To eliminate the second group I will have to enlarge the first group of warnings and
> the number of characters in single line will be bigger than 100.

Readability is preferred over getting rid of "line over 80 char" warning.
But it is maintainer's call here.

>>> ---
>>>
>>> Pawel Laszczak (31):
>>> usb: usbssp: Defined register maps and other useful structures.
>>> usb: usbssp: Added some decoding functions.
>>> usb: usbssp: Add trace events used in driver
>>> usb: usbssp: Added USBSSP platform driver
>>> usb: usbssp: Added first part of initialization sequence.
>>> usb: usbssp: added template functions used by upper layer.
>>> usb: usbssp: Initialization - added usbssp_mem_init
>>> usb: usbssp: Added ring and segment handling functions.
>>> usb: usbssp: add implementation of usbssp_mem_cleanup
>>> usb: usbssp: added usbssp_trb_in_td function.
>>> usb: usbssp: added function for stopping driver.
>>> usb: usbssp: added functions for queuing commands.
>>> usb: usbssp: addec procedure for handlin Port Status Change events.
>>> usb: usbssp: added procedure handling command completion events.
>>> usb: usbssp: added device controller error, transfer and SETUP
>>> completion event.
>>> usb: usbssp: added connect/disconnect procedures.
>>> usb: usbssp: added implementation of usbssp_halt_endpoint function.
>>> usb: usbssp: added handling of Port Reset event.
>>> usb: usbssp: added support for USB enumeration process.
>>> usb: usbssp: added queuing procedure for control transfer.
>>> usb: usbssp: added queuing procedure for BULK and INT transfer.
>>> usb: usbssp: added procedure removing request from transfer ring
>>> usb: usbssp: added implementation of transfer events.
>>> usb: usbssp: added detecting command timeout.
>>> usb: usbssp: added implementation of usbssp interface.
>>> usb: usbssp: added endpoint configuration functionality.
>>> usb: usbssp: implements usbssp_gadget_ep_enable function
>>> usb: usbssp: implemented usbssp_gadget_ep_disable function.
>>> usb: usbssp: added support for LPM.
>>> usb: usbssp: added support for TEST_MODE.
>>> usb: usbssp: add pci to platform driver wrapper.
>>>
>>> drivers/usb/Kconfig | 2 +
>>> drivers/usb/Makefile | 2 +
>>> drivers/usb/usbssp/Kconfig | 29 +
>>> drivers/usb/usbssp/Makefile | 15 +
>>> drivers/usb/usbssp/gadget-dbg.c | 39 +
>>> drivers/usb/usbssp/gadget-ep0.c | 571 +++++
>>> drivers/usb/usbssp/gadget-ext-caps.h | 102 +
>>> drivers/usb/usbssp/gadget-if.c | 569 +++++
>>> drivers/usb/usbssp/gadget-mem.c | 1938 +++++++++++++++
>>> drivers/usb/usbssp/gadget-port.c | 287 +++
>>> drivers/usb/usbssp/gadget-ring.c | 3452 ++++++++++++++++++++++++++
>>> drivers/usb/usbssp/gadget-trace.c | 13 +
>>> drivers/usb/usbssp/gadget-trace.h | 482 ++++
>>> drivers/usb/usbssp/gadget.c | 1909 ++++++++++++++
>>> drivers/usb/usbssp/gadget.h | 2373 ++++++++++++++++++
>>> drivers/usb/usbssp/usbssp-pci-wrap.c | 226 ++
>>> drivers/usb/usbssp/usbssp-plat.c | 186 ++
>>> 17 files changed, 12195 insertions(+) create mode 100644
>>> drivers/usb/usbssp/Kconfig create mode 100644
>>> drivers/usb/usbssp/Makefile create mode 100644
>>> drivers/usb/usbssp/gadget-dbg.c create mode 100644
>>> drivers/usb/usbssp/gadget-ep0.c create mode 100644
>>> drivers/usb/usbssp/gadget-ext-caps.h
>>> create mode 100644 drivers/usb/usbssp/gadget-if.c create mode 100644
>>> drivers/usb/usbssp/gadget-mem.c create mode 100644
>>> drivers/usb/usbssp/gadget-port.c create mode 100644
>>> drivers/usb/usbssp/gadget-ring.c create mode 100644
>>> drivers/usb/usbssp/gadget-trace.c create mode 100644
>>> drivers/usb/usbssp/gadget-trace.h create mode 100644
>>> drivers/usb/usbssp/gadget.c create mode 100644
>>> drivers/usb/usbssp/gadget.h create mode 100644
>>> drivers/usb/usbssp/usbssp-pci-wrap.c
>>> create mode 100644 drivers/usb/usbssp/usbssp-plat.c
>>>
>>
>> --
> cheers,
> Pawel
>

--
cheers,
-roger

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki