> >On Thu, Apr 20, 2023, at 11:00, Pawel Laszczak wrote: >> This patch introduces the main part of Cadence USBHS driver to Linux >> kernel. > >Not sure why I was on Cc, but I gave it a quick look anyway, looking for >common issues. I only found a few very minor things that can be improved, no >real problems: You were detected by get_maintainer.pl script :). I will remove you from cc in next version. ./scripts/get_maintainer.pl patch/0002-usb-cdns2-Add-main-part-of-Cadence-USBHS-driver.patch Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> (supporter:USB SUBSYSTEM,commit_signer:8/13=62%,commit_signer:4/6=67%) Pawel Laszczak <pawell@xxxxxxxxxxx> (maintainer:CADENCE USBHS DRIVER,commit_signer:1/6=17%,added_lines:1/4=25%) Arnd Bergmann <arnd@xxxxxxxx> (commit_signer:5/13=38%,authored:5/13=38%,added_lines:3/47=6%,removed_lines:31/47=66%,authored:1/6=17%,removed_lines:2/3=67%) Tony Lindgren <tony@xxxxxxxxxxx> (commit_signer:2/13=15%) Nicolas Ferre <nicolas.ferre@xxxxxxxxxxxxx> (commit_signer:1/13=8%) Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> (commit_signer:1/13=8%) Randy Dunlap <rdunlap@xxxxxxxxxxxxx> (authored:1/13=8%) Jean Delvare <jdelvare@xxxxxxx> (authored:1/13=8%) Biju Das <biju.das.jz@xxxxxxxxxxxxxx> (authored:1/13=8%,added_lines:13/47=28%,commit_signer:1/6=17%,authored:1/6=17%,added_lines:1/4=25%) Hans-Christian Noren Egtvedt <egtvedt@xxxxxxxxxxxx> (authored:1/13=8%) Neal Liu <neal_liu@xxxxxxxxxxxxxx> (added_lines:13/47=28%,commit_signer:1/6=17%,authored:1/6=17%,added_lines:1/4=25%) Herve Codina <herve.codina@xxxxxxxxxxx> (added_lines:11/47=23%,commit_signer:1/6=17%,authored:1/6=17%,added_lines:1/4=25%) Linus Walleij <linus.walleij@xxxxxxxxxx> (removed_lines:11/47=23%,authored:1/6=17%,removed_lines:1/3=33%) linux-kernel@xxxxxxxxxxxxxxx (open list) Thanks, Pawel > >> +++ b/drivers/usb/gadget/udc/cdns2/Kconfig >> @@ -0,0 +1,11 @@ >> +config USB_CDNS2_UDC >> + tristate "Cadence USBHS Device Controller" >> + depends on USB_PCI && ACPI && HAS_DMA > >Why the ACPI dependency? > >> + response_pkt = (__le16 *)pdev->ep0_preq.request.buf; >> + *response_pkt = cpu_to_le16(status); > >You can simplify this using put_unaligned_le16() > >> + >> + preq->num_of_trb = num_trbs; >> + >> + /* >> + * Memory barrier - cycle bit must be set as the last operation. >> + */ >> + wmb(); > >This can probably be the cheaper dma_wmb() if you only serialize between >accesses to a DMA buffer. > >> +static int __maybe_unused cdns2_pci_suspend(struct device *dev) { >> + struct cdns2_device *priv_dev = dev_get_drvdata(dev); >> + >> + return cdns2_gadget_suspend(priv_dev); } >> + >> +static int __maybe_unused cdns2_pci_resume(struct device *dev) { >> + struct cdns2_device *priv_dev = dev_get_drvdata(dev); >> + >> + return cdns2_gadget_resume(priv_dev, 1); } >> + >> +static const struct dev_pm_ops cdns2_pci_pm_ops = { >> + SET_SYSTEM_SLEEP_PM_OPS(cdns2_pci_suspend, cdns2_pci_resume) >}; > >You can use SYSTEM_SLEEP_PM_OPS() instead of >SET_SYSTEM_SLEEP_PM_OPS() and then remove the __maybe_unused. > > Arnd