linux-kernel - Re: [PATCH v2 00/10] Introduced new Cadence USBSSP DRD Driver.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20201106134136.GA3316286@kroah.com>
Date:   Fri, 6 Nov 2020 14:41:36 +0100
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Pawel Laszczak <pawell@...ence.com>
Cc:     peter.chen@....com, linux-usb@...r.kernel.org,
        linux-kernel@...r.kernel.org, balbi@...nel.org,
        colin.king@...onical.com, rogerq@...com, kurahul@...ence.com,
        nsekhar@...com
Subject: Re: [PATCH v2 00/10] Introduced new Cadence USBSSP DRD Driver.

On Fri, Nov 06, 2020 at 12:42:50PM +0100, Pawel Laszczak wrote:
> This patch introduce new Cadence USBSS DRD driver to linux kernel.
> 
> The Cadence USBSS DRD Controller 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 burned. We have support
> for PCIe bus, which is used on FPGA prototyping.
> 
> The host side of USBSS-DRD controller is compliance with XHCI
> specification, so it works with standard XHCI Linux driver.
> 
> The device side of USBSS DRD controller is compliant with XHCI.
> The architecture for device side is almost the same as for host side,
> and most of the XHCI specification can be used to understand how
> this controller operates.
> 
> This controller and driver support Full Speed, Hight Speed, Supper Speed
> and Supper Speed Plus USB protocol.
> 
> The prefix cdnsp used in driver has chosen by analogy to cdn3 driver.
> The last letter of this acronym means PLUS. The formal name of controller
> is USBSSP but it's to generic so I've decided to use CDNSP.
> 
> The patch 1: adds support for DRD CDNSP.
> The patch 2: separates common code that can be reusable by cdnsp driver.
> The patch 3: moves reusable code to separate module.
> The patch 4: changes prefixes in reusable code from cdns3 to common cdns.
> The patch 5: adopts gadget_dev pointer in cdns structure to make possible 
>              use it in both drivers.
> The patches 6-8: add the main part of driver and has been intentionally
>              split into 3 part. In my opinion such division should not
>              affect understanding and reviewing the driver, and cause that
>              main patch (7/8) is little smaller. Patch 6 introduces main
>              header file for driver, 7 is the main part that implements all
>              functionality of driver and 8 introduces tracepoints.
> The patch 9: Adds cdns3 prefixes to files related with USBSS driver.
> the patch 10: Adds USBSSP DRD IP driver entry to MAINTAINERS file.
> 
> Changlog from v1:
> - updated common code to latest cdns3 driver
> - moved cdnsp driver files to cdns3 as sugested by Peter Chan
> - removed duplicate code from cdnsp_ep0_set_config function
> - added cdns3 prefixes to file related with USBSS driver
> - updated MAINTAINERS file
> - fixed issue with U1
> - fixed issue with L1
> - some less improtant changes sugested by Chunfeng Yun

After a quick review, I don't see anything wrong with this series, nice
work.

It does feel odd you need to split things into a "common" and then 2
other modules, but I guess it makes sense.  Worst case, over time, you
merge them back together after everyone just ends up enabling both of
them :)

Felipe, want me to take these directly, or should they go through your
tree after you review them?  I never remember with this driver whose it
goes through.

thanks,

greg k-h